Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Elaborate/reference the "multiple threads are not an issue in Python 3.x" #5

Open
native-api opened this issue Mar 2, 2018 · 13 comments
Labels

Comments

@native-api
Copy link

native-api commented Mar 2, 2018

Wherever I look, everyone says that Tk/Tkinter has no facilities to call/send events into the main thread (even in Py3). So the only option is to poll some queue or something from it by repeatedly calling .after. And that mttkinter is the only real salvation.

So, stating that in Python 3.x, doing tkinter calls from another thread in not a problem as you do is quite a groundbreaking claim to make!
Do you have any references to back it with?

@RedFantom
Copy link
Owner

It has been a long time since I researched multi-threading with Tkinter, so please bear with me as I try to recall things as well as I can from a site that apparently is no longer available.

My only reference would be the statement in the wiki of this repository:

This module is only useful for Python 2.x installations, as in Python 3 Tkinter is compiled with a flag that ensures thread-safety.

I made this statement based on a forum post by the original author of the module, but unfortunately after spending the better part of an hour searching for the post, I concede that I can no longer find it. My leaky-as-a-cauldron memory will have to serve as a reference instead, I'm afraid. The forum post was posted in a thread that discussed mtTkinter in some form or another, and the original author stated that mtTkinter was not useful for Python 3, as in Python 3 Tcl is compiled with the thread-safe flag (which is, I believe, also the case for the latest Python 2 versions).

I understand if you find this a very weak explanation of why I stated what I did, and reading through my explanation, I cannot disagree with you if you do.

Going back down the rabbit-hole, I have looked up the archived version of the page I originally got the module from (as the original page is no longer available). My understanding, based on this and the reading that I did over a year ago, was as follows:

Tk is not thread-safe, unless it is built with the thread-safe flag. However, the Tcl-interpreter must also be built with the thread-safe flag to ensure thread-safety for Tkinter.

This understanding was supported by the fact that after I ported the program for which I initially used mtTkinter to Python 3 without mtTkinter, I no longer had any issues. However, given the supporting references you have provided, perhaps this understanding is wrong and it only worked in my case because of some weird coincidence.

The reproduction of threading issues in Tkinter is spotty, and it could be that even though I have used Threads multiple times now in Python 3 with Tkinter, I have just never encountered issue, and that doesn't have to mean that because I can't reproduce any issues with threading in Python 3 doesn't mean there aren't any (call it tunnel vision, if you will).

Conclusion
Reconsidering, I think that porting this module to Python 3 for anyone who does have issues with running apps using threading with Tkinter might be a good idea. The module is not big, and it should be easy enough to port over.
I will see if I can find the time in the upcoming week, but perhaps it'll have to wait longer. If anyone else feels up for it, feel free to open a Pull Request.

If you have any other questions or remarks, please don't hesitate to post them. I will get to them as soon as I can.

@RedFantom
Copy link
Owner

I have removed all references to Python 3 thread-safety from the Wiki and the README, so I'm going to close this issue now. Thank you for pointing this out to me, I wouldn't have discovered on my own.
If you have any other suggestions or questions, please share!

@native-api
Copy link
Author

native-api commented Mar 9, 2018

If the sources are not available, let's cut the middleman and check the claims directly.

the original author stated that mtTkinter was not useful for Python 3, as in Python 3 Tcl is compiled with the thread-safe flag (which is, I believe, also the case for the latest Python 2 versions).

https://github.com/python/cpython-source-deps/blob/tcl-8.5.19.0/win/makefile.vc#L73 (location deduced from get_externals.bat: py2.7 head):

OPTS=loimpact,msvcrt,static,staticpkg,symbols,threads,profile,unchecked,none

https://github.com/python/cpython-source-deps/blob/tcl-core-8.6.6.0/win/makefile.vc#L74 (py3.6 head):

OPTS=loimpact,msvcrt,nothreads,pdbs,profile,static,staticpkg,symbols,thrdalloc,tclalloc,unchecked,none

https://github.com/python/cpython/blob/master/PCbuild/tcl.vcxproj#L54 :
Options actually used are msvcrt, symbols (only for debug). For py2.7, same minus msvcrt. No other references to OPTS or TclOpts in the sourcetree. msvcrt seems to have no effect since there's no static.

So, 2.x does not build Tcl with thread support while 3.x does. What does that mean for "thread safety"?

Beginning with the 8.1 release, the Tcl core is thread safe, which
allows you to incorporate Tcl into multithreaded applications without
customizing the Tcl core. To enable Tcl multithreading support,
you must include the \fB-|-enable-threads\fR option to \fBconfigure\fR
when you configure and compile your Tcl core.
.PP
An important constraint of the Tcl threads implementation is that
\fIonly the thread that created a Tcl interpreter can use that
interpreter\fR. In other words, multiple threads can not access
the same Tcl interpreter. (However, a single thread can safely create
and use multiple interpreters.)

  • The thread option enables TCL_THREADS and TCL_THREAD_ALLOC preproc defines.

    • TCL_THREADS
      • enables mutexes in several places
      • in some subroutines, enables logic that checks if the interpreter called is associated with the current thread. and if not, forwards the call to the corresponding thread with ForwardOpToHandlerThread().
        • This effectively makes the "only the thread that created a Tcl interpreter can use that interpreter" limitation moot.
    • TCL_THREAD_ALLOC only enables custom malloc which "is a very fast storage allocator for used with threads (designed avoid lock contention)." It does have sync code but it's only enabled if both symbols are defined, so it doesn't affect thread safety on its own.
  • 2.x Tkinter still defines TCL_THREADS privately before including <tcl.h> (it allows it to use mutex procs, but they are a no-op in non-threaded Tcl). root.tk.call('array','get','tcl_platform') output doesn't include "threads" in 2.x. Thread support is checked with a Tcl call to the same effect. This rather suggests that both 2.x and 3.x are designed to work with both non-threaded and threaded Tcl.

So, it does look like threaded Tcl is thread safe in that it protects shared state with mutexes and allows unrestricted calls to interpreters associated with other threads.

It's not clear how much TKinter uses this though.

  • https://bugs.python.org/issue11077 was closed because the examples were shown to fail due to their rather than tkinter's bugs. I wasn't able to reproduce any crashes with TKinterCrash3.py but TKinterCrash2-2.py did crash fairly often.

Now, let's see the differences in Tkinter...

@native-api
Copy link
Author

native-api commented Mar 9, 2018

Sorry for the late reply. You can understand that this research took some time. I even cut it short -- before having analyzed Tkinter -- to be able to reply in a timely manner at all.

@RedFantom
Copy link
Owner

@native-api Thank you for your in-depth research into the specifics of the Tcl building process and how the thread-safety works! Research like that indeed takes time, and if you get around to also researching the Tkinter specifics, I would very much like to hear them.

That Tcl is designed to be thread-safe indeed does not guarantee that Tkinter is thread-safe inherently, and even if it is designed in such a manner, it might contain some implementation issues that people have encountered over time and thus resulted in weird issues that were solved when using a guarenteed thread-safe implementation.

@RedFantom RedFantom reopened this Mar 11, 2018
@native-api
Copy link
Author

In _tkinter changes 2->3, there are no relevant changes as shown below. So, the relevant logic is effectively the same.

As per _tkinter.diff.zip, the changes between 2 and 3 are:

  • Starting with Tcl 8.4, many APIs offer const-correctness.
    This means const qualifiers in many API signatures. Seems more a compile-time check enable rather than actual semantic change. Irrelevant.

  • The vast majority of differences is connected to the byte vs. Unicode str change and int vs long change. Others include: TkApp type change; int->Py_ssize_t; ditch custom malloc; accept lists as well as tuples in some functions; use Argument Clinic to parse arguments. Irrelevant.
  • Deleted TkApp_new baseName arg. Was unused anyway.
  • Removed unused & Tk-deprecated Tkapp_GlobalCall and a few others (https://bugs.python.org/issue5136). Irrelevant.
  • Removed module-global versions of dooneevent,mainloop,createfilehandler etc. Irrelevant since the recommended way is to call them via a tkapp.

This means that if there's a reason for discrepancy, it should lie in the relevant logic itself, probably resulting from the threaded vs non-threaded Tcl.

@native-api
Copy link
Author

native-api commented Apr 10, 2018

Okay, got the results on the logic, too.

All in all, for non-threaded Tcl, all Tcl calls are wrapped with locks (see comment at https://github.com/python/cpython/blob/master/Modules/_tkinter.c#L162 ) . So, they should be thread-safe and any race conditions are due to bugs.

And I did find such bugs: in Tkapp_Call and SetVar, Tcl lock is not held when creating/destroying Tcl objects for call arguments. The underlying functions change Tcl global heap state and things like the objects' reference counters -- intrinsically non-thread-safe operations protected by mutexes (which are no-ops in non-threaded Tcl).

The destruction in Tkapp_Call at least caused a double-free MSVC assertion error for me. I also confirmed the creation in the same fn to be causing another error: in TkinterCrash2-2.py example from https://bugs.python.org/issue11077 , create line Tk call randomly returns "None" (a string) which it's never supposed to according to its doc.

So, mttkinter is needed in Py2 as a workaround until those bugs are fixed (and in any versions older than the one having the fix).

In Py3, the example TkinterCrash3.py from the above ticket doesn't produce any errors whatsoever. And the functions seem to correctly forward calls to the interpreter's thread in accordance with the above-linked comment. So, in Py3, mttkinter isn't needed -- not because "Tkinter/Tcl in Py3 is thread-safe unlike in Py2", but because the bug in Tkinter is never triggered with threaded Tcl.

@native-api
Copy link
Author

@RedFantom
Copy link
Owner

Wow. Just wow. That is some very impressive an thorough research. I have actually encountered the bug you describe before, without even realizing it was related to this.

Thus, if I understand correctly, after your patch is merged mtTkinter will be obsolete for the updated version of Python 2.7? I will try to make some time to write a short explanation of the situation in the README with reference to this issue report, crediting you, of course, unless you would rather write it yourself as you have done the research.

Thank you for clearing this up! I really appreciate the deep dive and I think it is really great that you got to the bottom of this.

@native-api
Copy link
Author

native-api commented Apr 12, 2018

Thus, if I understand correctly, after your patch is merged mtTkinter will be obsolete for the updated version of Python 2.7?

Yes, exactly.

I've looked through all the other uses of the Tcl lock and run the example from the ticket in a loop for a few hours without a single error. So, no other affecting bugs in sight.

I will try to make some time to write a short explanation of the situation in the README with reference to this issue report, crediting you, of course

I'm okay with that. Since you don't need (and shouldn't) to go into too much detail, there's nothing I can do here that you can't. Moreover, since you saw the events from an outside perspective, it'll be easier for you to describe it in layman's terms.
You could make a pull request if you wish me to review the wording or something.

Repository owner deleted a comment Mar 22, 2019
@RedFantom
Copy link
Owner

@wekay102200 So, a few things before I actually get to your question:

  • Your issue is not related to this thread. Before you comment, please check whether your issue exactly matches what the title of an issue describes. If it does not, then you should open a new issue, especially if it is this old. This was a question and discussion on why I said in the README.md that Python 3 Tkinter is thread-safe, which turned out to be wrong.
  • Format your code properly for readability. Multi-line code is formatted like so.
  • If you do provide code samples of your problem, just like on StackOverflow, only Minimal and Verifiable examples work. Your code cannot be executed by anyone who finds it, so your problem cannot be verified. In fact, you do not even actually use multiple threads in the code you have provided.
  • Saying things like 'this is too troublesome' makes people less inclined to help you. Programming sometimes requires dirty solutions for dirty bugs, and it will always be an effort to make a big program do exactly what you want all the time. Be prepared for that.
  • Be specific with the information you provide. I know that you use Windows from the other issues, but you should list that every time, so that people other than me also know without having to go look for that information. You should also list your complete Python version, like Python 2.7.15rc1 64-bit and your full OS version, like Ubuntu 18.04 64-bit.

Then, on to your question:
If you were to read the full thread, then you would see that, while there is no summary, there is a lot of information that can help you. Specifically, pull-requests (if you do not know what a pull-request is, I recommend googling them. They are going to come up a lot on GitHub) on CPython are referenced and, were you to check them out, you would see that they have not been merged yet. Additionally, in the thread, you can read that actually the issues with multi-threading are caused by bugs, and that as the pull-requests have not been merged yet, those bugs must still be there. Therefore, if you use Python 2.7, then no, Tkinter cannot be used from multiple threads, and, by extension, none of its widgets are thread-safe.

There are three solutions:

  • Move to Python 3.x, where the bugs are not triggered
  • Use mtTkinter to wrap your non-thread-safe calls into a queue
  • Use the solution you already have, the 'troublesome' one

Now, I still want to leave this issue open so the information is easier to find. However, as your issue is not related to this thread, you should open another issue, if your issue is related to mtTkinter. For general programming questions, I really do recommend that you scour StackOverflow for answers, and possibly ask a question if there really is no answer to be found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants
@native-api @RedFantom and others