mainloop() is a method of a Tkapp object, but it's also a method of 
_tkinter module. In TkApp_MainLoop, self is seen as a TkappObject 
whereas it can be a module object! So instruction 
like "self->dispatch=1" will replace a random byte in the module 
object (eg. ob_type attribute). Example to crash:
  import gc
  import _tkinter
  _tkinter.mainloop()
  gc.collect()

It always crashs in my Python 3.0, but not in Python 2.6.

Solution: just remove _tkinter.mainloop() => it should have no side 
effect since users use tkinter (without the "_") which only uses 
Tkapp.mainloop() (the right way to use Tkapp_MainLoop() function). 
Solution "implemented" in the patch.

Looks fine to me. 
But I can't see the reason to keep this as a module function in python
2.6 so I would remove it there too.

gpolo agree to remove it. If no one is opposed for this change, can 
someone apply the patch?

"tkinter.mainloop" seems used in a bunch of places according to Google
Code. Am I missing something?

http://www.google.com/codesearch?hl=fr&lr=&q=%22tkinter.mainloop%22&sbtn=Rechercher

On Wed, Dec 31, 2008 at 2:24 PM, Antoine Pitrou <report@bugs.python.org> wrote:
>
> Antoine Pitrou <pitrou@free.fr> added the comment:
>
> "tkinter.mainloop" seems used in a bunch of places according to Google
> Code. Am I missing something?
>

Yes, those are not _tkinter.mainlooop, they are Tkinter.mainloop.

> http://www.google.com/codesearch?hl=fr&lr=&q=%22tkinter.mainloop%22&sbtn=Rechercher
>

On Wed, Dec 31, 2008 at 2:27 PM, Guilherme Polo <report@bugs.python.org> wrote:
>
> Guilherme Polo <ggpolo@gmail.com> added the comment:
>
> On Wed, Dec 31, 2008 at 2:24 PM, Antoine Pitrou <report@bugs.python.org> wrote:
>>
>> Antoine Pitrou <pitrou@free.fr> added the comment:
>>
>> "tkinter.mainloop" seems used in a bunch of places according to Google
>> Code. Am I missing something?
>>
>
> Yes, those are not _tkinter.mainlooop, they are Tkinter.mainloop.
>

Or were you referring to my other comment about removing it from
Tkinter.py too ?

Well, well, sorry for the noise!

The patch is incorrect. Tkapp_Mainloop is supposed to work both as a
module function and a method, and it tests for self to find out which
case it is. Now, this test is apparently broken in 3.x, but that could
be fixed, instead of ripping the module function out. Or, if that
function is removed, the code to support it should also be removed.

The same issue probably also affects other functions that double both as
instance methods and module functions.

> Tkapp_Mainloop is supposed to work both as a module function 
> and a method, and it tests for self to find out which
> case it is. Now, this test is apparently broken in 3.x, ...

Ok. In Python 2.x, selfptr is NULL whereas selfptr is a pointer to the 
module in Python 3.x. New attached patch uses PyModule_Check() to 
check if selfptr is the module or an object.

> if that function is removed, the code to support 
> it should also be removed.

Which code? I don't see which code uses _tkinter.mainloop. I saw code 
that calls the method mainloop() of a Tkapp object, like 
tkinter.mainloop() does. Or am I wrong?

> Ok. In Python 2.x, selfptr is NULL whereas selfptr is a pointer to the 
> module in Python 3.x. New attached patch uses PyModule_Check() to 
> check if selfptr is the module or an object.

The patch looks right in principle. Please make sure not to include
printf calls in it.

Please also consider all similar cases, such as Tkapp_CreateFileHandler.

>> if that function is removed, the code to support 
>> it should also be removed.
> 
> Which code? 

The code inside the function: all tests whether self is NULL. If
mainloop would stop being a module function, this code also should go.

Victor, you seem to be confusing "code that supports it" with "functions
that use it". There are some conditional code inside Tkapp_MainLoop that
depends on self being available, that is what Martin meant by code that
supports it, and since it would no longer be a module function that code
would no longer be needed.

Now, I'm much more in favour to remove it from moduleMethods than from
adjusting it to work in py3k. My reasons for the moment are: 

1) To me, it makes much more sense to call a mainloop function from a
Tcl interpreter object than from a module;
2) There is a member named dispatching in TkappObject, so I believe when
this tcl/tk bridge was created -- or at least when this member was added
-- it had the intention to allow multiple mainloops at some time (or is
it really only intended to be used when trying to grab the thread that
created the tcl interpreter ?);
3) It reduces code :)

Although the reason #2 won't just work after removing mainloop from the
module functions, it also doesn't help keeping it there.

> Now, I'm much more in favour to remove it from moduleMethods than from
> adjusting it to work in py3k.

Would you apply the same reasoning to the other Tkapp methods available
on _tkinter?

IIRC, these functions were there first; the Tkapp object was added
later. The module functions are kept for compatibility (which we
were free to break in 3.0).

I'm not opposed to removing these module functions now (not even for
3.0.1), but...

> 1) To me, it makes much more sense to call a mainloop function from a
> Tcl interpreter object than from a module;

To me, it makes perfect sense. Windowing events are a global, and not
specific to a Tcl interpreter - before receiving it, you cannot know
(in general) what interpreter it is for. Indeed, Tcl's Tcl_DoOneEvent
doesn't take a Tcl_Interp* parameter.

> 2) There is a member named dispatching in TkappObject, so I believe when
> this tcl/tk bridge was created -- or at least when this member was added
> -- it had the intention to allow multiple mainloops at some time (or is
> it really only intended to be used when trying to grab the thread that
> created the tcl interpreter ?);

This entire machinery got added when Tcl started supporting threads in
a way fundamentally different from Python's support for threads. Tcl
is inherently single-threaded, no GIL. To support threads, you create
a new interpreter for each new thread, and the different interpreters
are completely independent from each other. So when a Tkinter
application passes a Tcl command from one thread to another, this would
crash or otherwise not work.

In the old days, Python's threading could be used with Tcl just fine;
any thread could make Tk calls (on Unix, at least). When Tcl threading
was added, I tried to preserve this mode, by making one thread the
Tcl thread. Any other Python thread can't make direct Tcl calls (since
those would get to a different interpreter), but instead an RPC
system based on Tcl's thread synchronization was added. As this RPC
depends on the Tcl mainloop, the "dispatching" member tells the caller
if the Tcl thread is up.

So, no: the "dispatching" member is there for continued support of
a single mainloop, not for multiple mainloops.

> 3) It reduces code :)

That is a good reason. We would still need a complete patch.

> Victor, you seem to be confusing "code that supports it" with "functions
> that use it"

Oh ok, I didn't understood what "code that supports it" mean.

> There are some conditional code inside Tkapp_MainLoop that 
> depends on self being available, that is what Martin meant by code that
> supports it, and since it would no longer be a module function that code
> would no longer be needed.

Yes, right. I also prefer smaller code base with same functions ;-)

> Martin v. Löwis <martin@v.loewis.de> added the comment:
>
>> Now, I'm much more in favour to remove it from moduleMethods than from
>> adjusting it to work in py3k.
>
> Would you apply the same reasoning to the other Tkapp methods available
> on _tkinter?
>

As I see all those functions that are calling into Tcl should be moved
from the module functions. Some parts of the code inside these
functions are conflicting with each other, while one will try to allow
multiple threads, the other part will just disallow it. And since to
follow the Tcl appartment model we need to access some of the members
in TkappObject it just makes me want to remove those functions even
more.

> IIRC, these functions were there first; the Tkapp object was added
> later. The module functions are kept for compatibility (which we
> were free to break in 3.0).
>
> I'm not opposed to removing these module functions now (not even for
> 3.0.1), but...
>
>> 1) To me, it makes much more sense to call a mainloop function from a
>> Tcl interpreter object than from a module;
>
> To me, it makes perfect sense. Windowing events are a global, and not
> specific to a Tcl interpreter - before receiving it, you cannot know
> (in general) what interpreter it is for. Indeed, Tcl's Tcl_DoOneEvent
> doesn't take a Tcl_Interp* parameter.
>

That is way to see it. But what I was taking into account was
something like the proper update of the dispatching member for
different interpreters, this would mean dispatching could be used to
stop a running mainloop by setting it to 0.

>> 2) There is a member named dispatching in TkappObject, so I believe when
>> this tcl/tk bridge was created -- or at least when this member was added
>> -- it had the intention to allow multiple mainloops at some time (or is
>> it really only intended to be used when trying to grab the thread that
>> created the tcl interpreter ?);
>
> This entire machinery got added when Tcl started supporting threads in
> a way fundamentally different from Python's support for threads. Tcl
> is inherently single-threaded, no GIL. To support threads, you create
> a new interpreter for each new thread, and the different interpreters
> are completely independent from each other. So when a Tkinter
> application passes a Tcl command from one thread to another, this would
> crash or otherwise not work.
>

That is exactly one of the reasons to move mainloop and others from
the moduleMethods. It is hard for me to accept the two distinct
behaviours present in _tkinter, which are based from where you call
the function.

> In the old days, Python's threading could be used with Tcl just fine;
> any thread could make Tk calls (on Unix, at least). When Tcl threading
> was added, I tried to preserve this mode, by making one thread the
> Tcl thread. Any other Python thread can't make direct Tcl calls (since
> those would get to a different interpreter), but instead an RPC
> system based on Tcl's thread synchronization was added. As this RPC
> depends on the Tcl mainloop, the "dispatching" member tells the caller
> if the Tcl thread is up.
>

But I don't see a RPC being used there, I just see some polling.

> So, no: the "dispatching" member is there for continued support of
> a single mainloop, not for multiple mainloops.
>
>> 3) It reduces code :)
>
> That is a good reason. We would still need a complete patch.
>

> But I don't see a RPC being used there, I just see some polling.

Consider Tkapp_Call (e.g.). If this is invoked in the Tk interpreter
thread, then there is a direct call to Tcl_EvalObjv/Tkapp_CallResult.

If the call is made from a different thread, then a Tkapp_CallEvent
is allocated, filled with the parameters, and Tkapp_ThreadSend is
invoked. This puts the event into the thread queue of the receiving
thread, and waits for a condition.

In the interpreter thread, Tkapp_CallProc is invoked, which extracts
the arguments from the event, invokes Tcl_EvalObj/Tkapp_CallResult,
and notifies the condition.

> Martin v. Löwis <martin@v.loewis.de> added the comment:
>
>> But I don't see a RPC being used there, I just see some polling.
>
> Consider Tkapp_Call (e.g.). If this is invoked in the Tk interpreter
> thread, then there is a direct call to Tcl_EvalObjv/Tkapp_CallResult.
>
> If the call is made from a different thread, then a Tkapp_CallEvent
> is allocated, filled with the parameters, and Tkapp_ThreadSend is
> invoked. This puts the event into the thread queue of the receiving
> thread, and waits for a condition.
>
> In the interpreter thread, Tkapp_CallProc is invoked, which extracts
> the arguments from the event, invokes Tcl_EvalObj/Tkapp_CallResult,
> and notifies the condition.

This is all true but the dispatching isn't used there actually.
dispatching is being used in a polling manner to try to catch the
thread running the tcl interpreter which someone tried to call into,
the code then proceeds to do what you described.

> This is all true but the dispatching isn't used there actually.
> dispatching is being used in a polling manner to try to catch the
> thread running the tcl interpreter which someone tried to call into,
> the code then proceeds to do what you described.

Right. If the main thread doesn't actually invoke mainloop(), the
Tcl events don't get dispatched, and the RPC system breaks down,
effectively leading to a deadlock. To prevent application
breakage during startup, a grace period is added in case applications
create threads before starting the mainloop.

Here is a patch, two actually. The next one deprecates the functions in
trunk

These look fine. I think further changes are necessary:
tkinter/__init__.py tries to load createfilehandler/deletefilehandler;
this becomes redundant. Also, why did you leave DoOneEvent and Quit
as-is - don't they fall into the same category?

One minor nit: I personally dislike function pointer casts, because they
can easily go wrong. I rather prefer if the C compiler tells me when I
mess up with function pointer types. Hence _tkinter uses selfptr/self
all the time.

> These look fine. I think further changes are necessary:
> tkinter/__init__.py tries to load createfilehandler/deletefilehandler;
> this becomes redundant.

Indeed, I forgot to look into the Python code.

I also see as redundant the checks for _tkinter._flatten and
_tkinter._cnfmerge, the former because if we are still on Tkinter.py
(or __init__.py in py3k) then _tkinter exists and _flatten is there
too, the latter because there is no _cnfmerge in _tkinter.

> Also, why did you leave DoOneEvent and Quit
> as-is - don't they fall into the same category?
>

Ah, it is nice you ask this. I was indeed going to move then, but I
noticed strange things with DoOneEvent which made me only remove those
functions that were already checking the thread id of the caller and
the interp creator. But rethinking about this, it seems to make more
sense to do the move now and fix the "strange things" in other patch.
The problems I noticed may be a bit off-topic for this specific issue,
but I would hate to just say I had problems with a function and don't
say which errors I got, and how I got, so I have to write. So, the
problem started when I noticed DoOneEvent doesn't check for python
errors after Tcl_DoOneEvent returns, making me try to get one of those
SystemErrors "NULL result without error" -- I failed to get it, but I
got other unexpected results:

I get an empty string with the text below, but I was expecting some
nameerror message:

import _tkinter

def mybgerror(errmsg):
    print errmsg

called = []
def bad():
    called.append(True)
    raise InvalidThing

x = _tkinter.create()
x.createcommand("bad", bad)
x.createcommand("bgerror", mybgerror)
x.call("after", 1, "bad")

while not called:
    _tkinter.dooneevent()
_tkinter.dooneevent()

I ended up finding another problem (or problems, I forgot to annotate
them), like with call:

import _tkinter

def bad():
    raise InvalidThing

x = _tkinter.create()
x.createcommand("bad", bad)
x.call("bad")

Results in:

Traceback (most recent call last):
  File "my_badtests/test1.py", line 7, in <module>
    x.call("bad")
_tkinter.TclError

Meaning call is overwriting a python error by a tcl error, but since
there was no error in the tcl interpreter the error message was empty
(too many errors in a message).

But these all would deserve another(s) issues, so I will be moving
"quit" and "dooneevent" from there too.

> One minor nit: I personally dislike function pointer casts, because they
> can easily go wrong. I rather prefer if the C compiler tells me when I
> mess up with function pointer types. Hence _tkinter uses selfptr/self
> all the time.
>

Fine.

> I get an empty string with the text below, but I was expecting some

s/text/test

> But these all would deserve another(s) issues, so I will be moving
> "quit" and "dooneevent" from there too.

I haven't tried reproducing these problems, but this all sounds plausible.

So go ahead and check in all the changes for this issue.

Don't forget Misc/NEWS entries, and don't forget to svnmerge block
the 2.7 change from being merged into 3.k. You should then also
merge the 3k change into the 3.0 branch. I don't think we have
to merge the warnings into 2.6 (but could if we wanted to).
Leave the various revision numbers in this report when done.

If you rather prefer me to check it in, please let me know.

Thanks Martin,

trunk: r68231 (blocked on py3k)
py3k: r68237
release30-maint: r68239

Uh, I forgot about the code removal in __init__ in the first commits.

r68242, r68244 now

gpolo: Nice patches, good job and thanks ;-)

See Issue 22155 for fixing the createfilehandler FAQ documentation