classification
Title: revise Tkinter mainloop dispatching flag behavior
Type: enhancement Stage: patch review
Components: Tkinter Versions: Python 3.10
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Richard Sheridan, epaine, gpolo, serhiy.storchaka, taleinat
Priority: normal Keywords: patch

Created on 2020-06-30 20:44 by Richard Sheridan, last changed 2020-07-13 08:41 by epaine.

Files
File name Uploaded Description Edit
waitmainloop.py epaine, 2020-07-01 10:15
Pull Requests
URL Status Linked Edit
PR 21299 open Richard Sheridan, 2020-07-03 19:55
Messages (7)
msg372722 - (view) Author: Richard Sheridan (Richard Sheridan) * Date: 2020-06-30 20:44
This could also be considered a "behavior" type issue.

`TkappObject` has a member `dispatching` that could usefully be exposed by a very simple read-only method for users to determine at runtime if the tkinter mainloop is running. Matplotlib and I'm sure other packages rely on fragile hacks (https://github.com/matplotlib/matplotlib/blob/a68562aa230e5895136120f5073dd01f124d728d/lib/matplotlib/cbook/__init__.py#L65-L71) to determine this state. I ran into this in https://github.com/matplotlib/matplotlib/pull/17802. All these projects would be more reliable with a new "dispatching()" method on the TkappObject, tkinter.Misc objects, and possibly the tkinter module itself.

Internally, `dispatching` is used to, yes, determine if the mainloop is running. However, this determination is always done within the `WaitForMainloop` function (https://github.com/python/cpython/blob/bd4a3f21454a6012f4353e2255837561fc9f0e6a/Modules/_tkinter.c#L363-L380), which waits up to 1 second for the mainloop to come up. Apparently, this function allows a thread to implicitly wait for the loop to come up by calling any `TkappObject` method. This is a bad design choice in my opinion, because if client code wants to start immediately and the loop is not started by mistake, there will be a meaningless, hard-to-diagnose delay of one second before crashing. Instead, if some client code in a thread needs to wait for the mainloop to run, it should explicitly poll `dispatching()` on its own. This waiting behavior should be deprecated and, after a deprecation cycle perhaps, all `WaitForMainloop()` statements should be converted to inline `self->dispatching`.

The correctness of the `dispatching` flag is dampened by the currently existing, undocumented `willdispatch` method which simply arbitrarily sets the `dispatching` to 1. It seems `willdispatch` was added 18 years ago to circumvent a bug building pydoc caused by `WaitForMainloop` not waiting long enough, as it tricks `WaitForMainloop` into... not waiting for the mainloop. This was in my opinion a bad choice in comparison to adding a dispatching flag: again, if some thread needs to wait for the mainloop, it should poll `dispatching()`, and avoid adding spurious 1 second waits. `willdispatch` currently has no references in CPython and most GitHub references are to Pycharm stubs for the CPython method. It should be deprecated and removed to preserve the correctness of `dispatching`.

Happy to make a PR about this, except I don't understand clinic at all, nor the specifics of deprecation cycles in CPython.
msg372741 - (view) Author: E. Paine (epaine) * Date: 2020-07-01 10:15
I agree it would be helpful to expose an explicit way of telling if the mainloop was running but am not sure about removing `WaitForMainloop` as it could very easily break existing programs.

If a program executes a tkinter method in a thread before the mainloop is executed, the method will wait because of the call to `WaitForMainloop`. In the example script this is done deliberately to demonstrate the behaviour but could be done accidentally if the main thread has to do something else before the mainloop (and after the thread has been created).

I think the changes (whatever is concluded we should do) would be considered an 'enhancement', which would not be backported to 3.9 and before (I believe 'behaviour' is generally used for logic errors).

I am very willing to help review a PR, however the people you really need to convince are Serhiy and/or Guilherme (I have added them to the nosy).
msg372749 - (view) Author: Richard Sheridan (Richard Sheridan) * Date: 2020-07-01 11:52
Removing `WaitForMainloop` would surely break some existing programs, but
that's why I suggested deprecation instead of just removing it suddenly. We
could issue a RuntimeWarning if `WaitForMainloop` actually waits and tell
the client to take responsibility for the race condition they created.
(They may have no idea! What if their delay unexpectedly increases to 1.2
seconds?) Whether or not waiting gets deprecated, it would make sense to
make the sleep behavior configurable instead of hardcoded. I'll include
something along those lines in my PR.

On Wed, Jul 1, 2020 at 6:15 AM E. Paine <report@bugs.python.org> wrote:

>
> E. Paine <paineelisha@gmail.com> added the comment:
>
> I agree it would be helpful to expose an explicit way of telling if the
> mainloop was running but am not sure about removing `WaitForMainloop` as it
> could very easily break existing programs.
>
> If a program executes a tkinter method in a thread before the mainloop is
> executed, the method will wait because of the call to `WaitForMainloop`. In
> the example script this is done deliberately to demonstrate the behaviour
> but could be done accidentally if the main thread has to do something else
> before the mainloop (and after the thread has been created).
>
> I think the changes (whatever is concluded we should do) would be
> considered an 'enhancement', which would not be backported to 3.9 and
> before (I believe 'behaviour' is generally used for logic errors).
>
> I am very willing to help review a PR, however the people you really need
> to convince are Serhiy and/or Guilherme (I have added them to the nosy).
>
> ----------
> nosy: +epaine, gpolo, serhiy.storchaka
> versions:  -Python 3.5, Python 3.6, Python 3.7, Python 3.8, Python 3.9
> Added file: https://bugs.python.org/file49283/waitmainloop.py
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue41176>
> _______________________________________
>
msg373029 - (view) Author: Richard Sheridan (Richard Sheridan) * Date: 2020-07-05 12:30
I'd like to consider one more possibility for future behavior that sort of came to mind while discussing the PR. In current behavior, it is possible to use `willdispatch` to trick `WaitForMainloop` into letting a thread pass through the timeout, where it will eventually wait on a `Tcl_ConditionWait` in `Tkapp_ThreadSend`. 

This could be very efficient default behavior, since no polling is required; the thread just goes when the loop comes up. Is it possible to make this a well-documented feature and default behavior of tkinter? Or would it be too surprising for new and existing users? It would be important to make sure that threads aren't silently getting lost in old programs and new users can figure out they need to call `mainloop`, `doonevent`, or `update` when not on the REPL or the thread will hang.
msg373152 - (view) Author: Richard Sheridan (Richard Sheridan) * Date: 2020-07-06 17:42
I'm planning to write the long-awaited Tkinter Internals section of the docs. (https://github.com/python/cpython/blame/master/Doc/library/tk.rst#L48) I've spent too much time at this point to let it all go down the memory hole. Unfortunately, I don't know how ALL of the internals work. Is there someone else we could add to nosy that might be interested in writing some subsections?

Also, should this extended docs contribution be a new issue or rolled in with this one? Much abbreviated documentation of the new methods in this PR could be added to tkinter.rst. The new docs issue would be dependent on this issue since I won't be able to complete the docs until we have finished discussing what the future behavior of threads waiting for `dispatching` will be (error & poll vs Tcl_ConditionWait).
msg373568 - (view) Author: E. Paine (epaine) * Date: 2020-07-12 19:59
I have just finished reviewing the proposed PR, and am happy with the content. During the process of developing the PR, we established that the behaviour that should be deprecated is the error after a second of waiting in a thread. Instead, on `WaitForMainloop` removal, we should pass straight through and use the tcl queue to wait for mainloop.

@Serhiy, is waiting for the tcl queue acceptable behaviour? It seemed to behave correctly during my tests and if it is acceptable, both me and Richard would really appreciate your review of the PR.
msg373587 - (view) Author: E. Paine (epaine) * Date: 2020-07-13 08:41
Apologies, it is not waiting for the tcl queue and instead the call waits indefinitely for `Tcl_ConditionWait` (tkinter adds it to the queue and then waits for the command to finish executing). Do we need some mechanism to alert people after a second, for example, that the thread is waiting for the mainloop to come up?
History
Date User Action Args
2020-07-13 08:41:46epainesetnosy: + taleinat
messages: + msg373587
2020-07-12 19:59:55epainesetmessages: + msg373568
2020-07-06 17:42:49Richard Sheridansetmessages: + msg373152
2020-07-05 12:30:07Richard Sheridansetmessages: + msg373029
2020-07-03 19:55:44Richard Sheridansetkeywords: + patch
stage: patch review
pull_requests: + pull_request20448
2020-07-01 11:52:04Richard Sheridansetmessages: + msg372749
2020-07-01 10:15:03epainesetfiles: + waitmainloop.py
versions: - Python 3.5, Python 3.6, Python 3.7, Python 3.8, Python 3.9
nosy: + gpolo, serhiy.storchaka, epaine

messages: + msg372741
2020-06-30 20:44:35Richard Sheridancreate