Skip to content

test_tcl and test_ttk_guionly don't like each other #49372

Description

@gpolo
mannequin
BPO 5122
Nosy @loewis, @ericvsmith
Files
  • dont_load_tk_in_tkintercreate.diff
  • protect_tk_loading.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2009-02-09.20:55:15.696>
    created_at = <Date 2009-02-01.04:45:08.400>
    labels = ['expert-tkinter']
    title = "test_tcl and test_ttk_guionly don't like each other"
    updated_at = <Date 2009-02-09.20:55:15.694>
    user = 'https://bugs.python.org/gpolo'

    bugs.python.org fields:

    activity = <Date 2009-02-09.20:55:15.694>
    actor = 'gpolo'
    assignee = 'none'
    closed = True
    closed_date = <Date 2009-02-09.20:55:15.696>
    closer = 'gpolo'
    components = ['Tkinter']
    creation = <Date 2009-02-01.04:45:08.400>
    creator = 'gpolo'
    dependencies = []
    files = ['12955', '12971']
    hgrepos = []
    issue_num = 5122
    keywords = ['patch']
    message_count = 17.0
    messages = ['80901', '81272', '81273', '81276', '81279', '81280', '81290', '81293', '81295', '81298', '81299', '81304', '81308', '81346', '81350', '81486', '81488']
    nosy_count = 3.0
    nosy_names = ['loewis', 'eric.smith', 'gpolo']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue5122'
    versions = ['Python 3.1', 'Python 2.7']

    Activity

    1. gpolo commented on Feb 1, 2009

      gpolomannequin
      MannequinAuthor

      Recently I've added this test named test_ttk_guionly, and I noticed some
      buildslaves went red, like amd64 gentoo trunk and x86 FreeBSD trunk (I
      though there were others for the reason I'm going to say next, but
      didn't found on a quick look).

      amd 64 gentoo trunk is failing with:

      .
      .
      test_ttk_guionly
      test_ttk_guionly skipped -- ttk not available: no display name and no
      $DISPLAY environment variable
      .
      .
      test_tcl

      remoteFailed: [Failure instance: Traceback (failure with no frames):
      twisted.internet.error.ConnectionLost: Connection to the other side was
      lost in a non-clean fashion.
      ]

      When it is not test_tcl, it is test_ttk_guionly.
      I tried duplicating the issue here by creating a buildmaster and a
      buildslave, used same options to build and to run tests, also tried
      enabling and disabling pty (I've found some old emails about problems
      involving pty), but couldn't duplicate it.

      So I followed a build on this buildslave "amd 64 gentoo trunk" and
      noticed it took a long time from the time it started running test_tcl
      till when the error appeared, which makes me imagine there is some
      problem regarding X server authorization. Now this is when I started
      checking how many other slaves were failing with the same error, and
      then I found this "x86 FreeBSD trunk" which failed with the following
      message:

      .
      .
      test_ttk_guionly
      test_ttk_guionly skipped -- ttk not available: can't find package tile
      .
      .
      test_tcl
      XIO: fatal IO error 9 (Bad file descriptor) on X server ":0.0"

        after 25 requests (23 known processed) with 1 events remaining.
      

      *** Error code 1

      This second bot had DISPLAY set, while the first didn't (although its
      TERM is set to xterm). I wonder if the first error ends up being the
      same as this second one.

      I'm asking myself if test_ttk_guionly is so wrong that it should be
      disabled everywhere, and even if it gets disabled, what if in the future
      another test comes in and "conflicts" with test_tcl ?

    2. gpolo commented on Feb 6, 2009

      gpolomannequin
      MannequinAuthor

      Eric, I noticed you said Fedora 6, so I checked which tcl/tk it
      includes and apparently it is 8.4.13, is that correct ?
      I tried using tcl/tk 8.4.13 and I managed to hang it too :)

      Try running test_tcl followed by test_ttk_guionly with regrtest.

    3. ericvsmith commented on Feb 6, 2009

      @ericvsmith
      Member

      Yes, I have these installed:
      tcl-8.4.13-3.fc6
      tcl-devel-8.4.13-3.fc6
      tk-8.4.13-3.fc6
      tk-devel-8.4.13-3.fc6

      When I run "./python Lib/test/regrtest.py test_tcl test_ttk_guionly", it
      hangs.

    4. gpolo commented on Feb 6, 2009

      gpolomannequin
      MannequinAuthor

      When I run "./python Lib/test/regrtest.py test_tcl test_ttk_guionly", it
      hangs.

      I have isolated it now:

      Traceback (most recent call last):
        File "<stdin>", line 1, in <module>
        File "/home/gpolo/python-dev/python-trunk/Lib/lib-tk/Tkinter.py",
      line 1649, in loadtk
          self.tk.loadtk()
      _tkinter.TclError: no display name and no $DISPLAY environment variable
      >>> Tkinter.Tk()
      hang!

      Very nice, much more "solvable" now after getting to know the tcl/tk version.

    5. gpolo commented on Feb 6, 2009

      gpolomannequin
      MannequinAuthor

      After "I have isolated it now:", there should be this interactive
      session (but gmail ate it apparently):

      >>> import os
      >>> del os.environ['DISPLAY']
      >>> import Tkinter
      >>> t = Tkinter.Tcl()
      >>> t.loadtk()
      Traceback (most recent call last):
       File "<stdin>", line 1, in <module>
       File "/home/gpolo/python-dev/python-trunk/Lib/lib-tk/Tkinter.py",
      line 1649, in loadtk
         self.tk.loadtk()
      _tkinter.TclError: no display name and no $DISPLAY environment variable
      >>> Tkinter.Tk()
      hang!
    6. gpolo commented on Feb 6, 2009

      gpolomannequin
      MannequinAuthor

      Ok, I've found the cause of the problem and the patch attached should
      solve it but shouldn't be applied! This was just a quick fix I did to
      confirm what I expected, I will add something more correct later.

      _tkinter is aware of this deadlock that we are getting (see
      Tkapp_TkInit), but it is not collaborating to solve it. The problem is
      avoided if we always use "loadtk" in Tkinter.py instead of using
      "create" to load tk.

    7. ericvsmith commented on Feb 6, 2009

      @ericvsmith
      Member

      The patch does solve the problem for me. It no longer hangs when running
      either:
      ./python Lib/test/regrtest.py test_tcl test_ttk_guionly
      or:
      ./python Lib/test/regrtest.py

      -------------
      ...
      test_traceback
      test_transformer
      test_ttk_guionly
      test_ttk_guionly skipped -- ttk not available: Calling Tk_Init again
      after a previous call failed might deadlock
      test_ttk_textonly
      test_tuple
      ...
      -------------

      Thanks for tracking this down.

    8. gpolo commented on Feb 6, 2009

      gpolomannequin
      MannequinAuthor

      Here is a better patch, for some definition of better.

      It syncs the failed attempt to load tk that may occur in
      _tkinter.Tkapp_TkInit, _tkinter.Tcl_AppInit and tkappinit.Tcl_AppInit.
      And that is done only for tk < 8.4.14 now.

      Said that, I'm a bit worried about the (new macro) TK_VERSION_HEX in
      _tkinter and tkappinit, maybe it is time to add a _tkinter.h ?

    9. gpolo commented on Feb 6, 2009

      gpolomannequin
      MannequinAuthor

      Any chance you can look at the proposed patch, Martin ? I found out that
      it was you who reported this deadlock back then, so you might want to
      review the patch.

    10. ericvsmith commented on Feb 6, 2009

      @ericvsmith
      Member

      The second patch (checking_for_failed_tk_load.diff) also works for me.

    11. gpolo commented on Feb 6, 2009

      gpolomannequin
      MannequinAuthor

      Found a place still out of sync, it is fixed on this new patch. To
      reproduce the problem with the previous patch try this:

      import os
      del os.environ['DISPLAY']
      import Tkinter
      Tkinter.Tk()
      Tkinter.Tcl().loadtk() # should hang now (with a proper tk version)
    12. ericvsmith commented on Feb 6, 2009

      @ericvsmith
      Member

      With the second patch installed, your code snippet does indeed hang for me.

      With the third patch installed, I get:
      [trunk]$ ./python 
      Python 2.7a0 (trunk:69369M, Feb  6 2009, 14:59:32) 
      [GCC 4.1.2 20070626 (Red Hat 4.1.2-13)] on linux2
      Type "help", "copyright", "credits" or "license" for more information.
      >>> import os
      >>> del os.environ['DISPLAY']
      >>> import Tkinter
      Tkinter.Tk()
      >>> Tkinter.Tk()
      Traceback (most recent call last):
        File "<stdin>", line 1, in <module>
        File "/root/python/trunk/Lib/lib-tk/Tkinter.py", line 1643, in __init__
          self.tk = _tkinter.create(screenName, baseName, className,
      interactive, wantobjects, useTk, sync, use)
      _tkinter.TclError: no display name and no $DISPLAY environment variable
      >>> Tkinter.Tcl().loadtk() # should hang now (with a proper tk version)
      Traceback (most recent call last):
        File "<stdin>", line 1, in <module>
        File "/root/python/trunk/Lib/lib-tk/Tkinter.py", line 1649, in loadtk
          self.tk.loadtk()
      _tkinter.TclError: Calling Tk_Init again after a previous call failed
      might deadlock

      Which I assume is the expected behavior.

      Also with the third patch installed, the regrtest's all pass.

    13. gpolo commented on Feb 6, 2009

      gpolomannequin
      MannequinAuthor

      Eric Smith added the comment:
      .
      .
      Which I assume is the expected behavior.

      Yes it is expected.

      Also with the third patch installed, the regrtest's all pass.

      Very nice.

      And Eric, thanks for the patience and for testing them all.

    14. gpolo commented on Feb 7, 2009

      gpolomannequin
      MannequinAuthor

      A bit cleaner patch.

    15. ericvsmith commented on Feb 7, 2009

      @ericvsmith
      Member

      protect_tk_loading.diff works for me.

    16. ericvsmith commented on Feb 9, 2009

      @ericvsmith
      Member

      The current patch (protect_tk_loading.diff) looks reasonable to me, and
      it solves my problem with the tests hanging. So I'd suggest you commit
      the patch, insofar it improves on the current situation.

      I'm not an expert in tk, however. So if you're looking for advice from
      someone knowledgeable on tk's inner workings, you'll need to get someone
      else involved.

    17. gpolo commented on Feb 9, 2009

      gpolomannequin
      MannequinAuthor

      Thanks Eric, committed in r69473.

    18. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions