revise Tkinter mainloop dispatching flag behavior

[3925c639-8ebd-4a44-a86e-a081cbca77ae]

BPO	41176
Nosy	@taleinat, @serhiy-storchaka, @E-Paine, @richardsheridan
PRs	python/cpython#21299
Files	waitmainloop.py

^{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: ```python assignee = None closed_at = None created_at = labels = ['type-feature', 'expert-tkinter', '3.10'] title = 'revise Tkinter mainloop dispatching flag behavior' updated_at = user = 'https://github.com/richardsheridan' ``` bugs.python.org fields: ```python activity = actor = 'epaine' assignee = 'none' closed = False closed_date = None closer = None components = ['Tkinter'] creation = creator = 'Richard Sheridan' dependencies = [] files = ['49283'] hgrepos = [] issue_num = 41176 keywords = ['patch'] message_count = 7.0 messages = ['372722', '372741', '372749', '373029', '373152', '373568', '373587'] nosy_count = 5.0 nosy_names = ['taleinat', 'gpolo', 'serhiy.storchaka', 'epaine', 'Richard Sheridan'] pr_nums = ['21299'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue41176' versions = ['Python 3.10'] ```

[3925c639-8ebd-4a44-a86e-a081cbca77ae]

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.

[919475b3-36a2-4cd7-997c-9c38f05f93c7]

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).

[3925c639-8ebd-4a44-a86e-a081cbca77ae]

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\

---

[3925c639-8ebd-4a44-a86e-a081cbca77ae]

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.

[3925c639-8ebd-4a44-a86e-a081cbca77ae]

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).

[919475b3-36a2-4cd7-997c-9c38f05f93c7]

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.

[919475b3-36a2-4cd7-997c-9c38f05f93c7]

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?