Menu

#3094 XFilterEvent() hangs

current: 8.6.0
open
9
2013-05-23
2013-05-21
Don Porter
No

Starting with checkin

http://core.tcl.tk/tk/info/2a535c6792

we've started calling XInitThreads() as part of
opening the first display in a process, which is
meant to improve multi-threaded access to
the X server.

However, since then, the first call to XFilterEvent()
in tclUnixEvent.c that has a Window argument with
a value other than None will hang up the application
hard. It becomes non-responsive, and has to be
killed. Apparently a deadlock inside the Xlib brings
things to a halt.

Typically this is the first key press in the application
that triggers the hang.

Discussion

  • Don Porter

    Don Porter - 2013-05-21

    I'm able to trigger the problem using just the demo
    programs included with Tk.

    It's quite possible the real problem here is a seriously
    buggy Xlib; I'm searching for that evidence now.

     
  • Don Porter

    Don Porter - 2013-05-21

    $ make shell
    ...
    % source $tk_library/demos/widget
    <Press a key>

    That locks things up on my CentOS 5.9
    system.

    It reports that X.Org libX11, Version 1.0.3, Release 11.el5_7.1
    is installed.

     
  • Don Porter

    Don Porter - 2013-05-21

    https://bugzilla.redhat.com/show_bug.cgi?id=743679

    Looks like reports of Qt suffering the same troubles.

    It appears that the decision to have Tk start calling
    XInitThreads() is a decision to start trusting the X11
    library in use (and its extensions, SCIM I'm looking
    at you) to have its threading bugs shaken out. There
    may be systems where that's a good bet, but it's looking
    like many systems still in use, that's not a good bet at
    all. What should we do to not make the next release
    of Tk useless on those systems?

     
  • Don Porter

    Don Porter - 2013-05-21

    http://bugs.winehq.org/show_bug.cgi?id=6547

    Record of the Wine project suffering through what
    seems like the same problems in 2006-2007.
    Mentions some "workaround" applied for the Wine 0.9.35
    release, but I haven't yet found what that might be.

     
  • Don Porter

    Don Porter - 2013-05-21

    Testing on a CentOS 6 systems, things work just
    fine, which lends support to the conclusion that
    the real problem here is threading bugs in old X11
    libraries.

    How old of X11 libraries does Tk wish to support, and
    what measure can Tk take at build time and/or install time
    to detect X11 libraries too old to use and report that?

     
  • Joe English

    Joe English - 2013-05-21

    Deadlock replicated here as well; not using SCIM or any other external IM, just the default built-in one.

    The initial report that prompted calling XInitThreads() in the first place was:

    http://mid.gmane.org/2AB6A31A-743B-4B35-AAEC-FC4BF546627F@mentor.com

    (Summary: bug manifested in a custom embedded app (not wish, and nobody else reported seeing the same issue), calling XInitThreads seems to have fixed the problem. Also worth noting that the app was single-threaded, so XInitThreads should not in theory have even been necessary.)

    Reverting http://core.tcl.tk/tk/info/2a535c6792 seems wise. The initial assesment that calling XInitThreads() is harmless and might be helpful is demonstrably incorrect.

     
  • Don Porter

    Don Porter - 2013-05-21

    Just one more facet of this matter:

    The Tk test suite was completely ineffective
    in bringing this problem to anyone's attention.
    I'm guessing because real KeyPress events
    are required to make the trouble surface and
    the test suite uses only the virtual kind?

     
  • Joe English

    Joe English - 2013-05-21

    More info: XInitThreads() is only necessary if multple threads use Xlib concurrently, which Tk does not do. We advise Tk users not to do that either.

     
  • Jan Nijtmans

    Jan Nijtmans - 2013-05-22

    >How old of X11 libraries does Tk wish to support, and
    >what measure can Tk take at build time and/or install time
    >to detect X11 libraries too old to use and report that?
    It looks like XInitThreads() should only be used on the
    newest X11 libraries. Well, In some very new X11 version,
    XKeycodeToKeysym() became deprecated, that's something
    we could test for. See:
    <http://core.tcl.tk/tk/info/67d17edf28>

    Would that be acceptable, or should we simply
    put #if 0 around this section and defer this to a
    later Tk version? Or is there maybe a better
    (more conservative) check that can be done?
    I will not object either way: this
    problem appears to be too serious.

    Regards,
    Jan Nijtmans

     
  • Don Porter

    Don Porter - 2013-05-22

    Thanks for the bug-3613668 branch.

    I confirm that if I rebuild Tk from the ground up
    from that branch, then my lockup problems go
    away on my CentOS 5.9 system.

    In this case the test for the Keysym deprecation
    is an effective proxy for what we really need to know.
    A more direct test would be more desirable.

    Not clear to me whether a configure-time detection
    like this solves the whole problem. For someone
    who ships binaries containing Tk that expect to link
    up with X11 libraries on the target system, I don't know
    that this takes care of the matter.

    As a minor point, I don't see the point of the
    #if defined(TCL_THREADS) guard. Nothing
    else in Tk uses this.

     
  • Don Porter

    Don Porter - 2013-05-22

    http://lists.x.org/archives/xorg-announce/2010-November/001548.html

    Appears that the X.org 1.4.0 release of libX11 might be a good
    dividing point to attempt to make the distinction on. The release
    notes indicate that's when the "xcb" (which was the source of
    Brian's original troubles) became a non-optional part.

     
  • Jan Nijtmans

    Jan Nijtmans - 2013-05-23
    • status: open --> pending-fixed
     
  • Jan Nijtmans

    Jan Nijtmans - 2013-05-23

    >In this case the test for the Keysym deprecation
    >is an effective proxy for what we really need to know.
    >A more direct test would be more desirable.
    Fully agreed. Merged to trunk now, so it can get
    more wide testing. Changing it to #ifdef 0 can
    always be done if this solution turns out not
    to be sufficient.

    >As a minor point, I don't see the point of the
    >#if defined(TCL_THREADS) guard. Nothing
    >else in Tk uses this.
    This is needed because:
    1) The variable xinitMutex would not even exist
    when Tk is compiled without threads, so the
    code would not even compile then.
    2) If Tcl is compiled with threads, but Tk is
    compiled without (however unlikely) we
    don't want to call XInitThreads either.

    Please evaluate and test!

     
  • Don Porter

    Don Porter - 2013-05-23
    • status: pending-fixed --> open
     
  • Don Porter

    Don Porter - 2013-05-23

    That's not correct. Tk has other Tcl_Mutex
    values that it locks and unlocks and none
    of them are, or need to be, guarded with a

    #ifdef TCL_THREADS

    Furthermore, TCL_THREADS does not
    tell you whether the Tcl library linked
    against was compiled with threads enabled.
    You'd have to query the runtime with something
    like [::tcl::pkgconfig get threaded] to find that out.
    TCL_THREADS tells you whether Tk was configured
    with --enable-threads, which it always should be.

     
  • Jan Nijtmans

    Jan Nijtmans - 2013-05-23

    >You'd have to query the runtime with something
    >like [::tcl::pkgconfig get threaded] to find that out.
    Well, I built in an easier check than using
    ::tcl::pkgconfig, which doesn't work here because
    we have an "interp" available here.
    See:
    <http://core.tcl.tk/tk/artifact/8246bd3ed3?ln=140-143>
    The comments here explain the trick.

    >TCL_THREADS tells you whether Tk was configured
    >with --enable-threads, which it always should be.
    Yes, it always should be, but the user still has the
    option to compile Tcl and Tk with --disable-threads.
    That case should still work.

     
  • Don Porter

    Don Porter - 2013-05-23

    I see the trickery now. And for that the
    TCL_THREADS does need to be defined.

     
  • Jan Nijtmans

    Jan Nijtmans - 2013-05-24

    It turns out that the test which checked whether "XKeycodeToKeysym" had two problems:
    - On non-gcc compilers which don't have -Werror, this might be mis-interpreted as deprecation.
    - The test result yes/no was reversed.
    Fixed this now, but it also means that the dgp's results are
    worthless ;-(, therefore disabled it now until someone
    has another bright idea. Anyone?