| msg31833 - (view) |
Author: Andrew Bennetts (spiv) |
Date: 2007-04-19 08:22 |
| This script always fails for me, usually on the very first iteration: ---- import threading while True: print '.', t = threading.Thread(target=lambda: None) t.start() t.join() alive_threads = threading.enumerate() assert len(alive_threads) == 1, alive_threads ---- The docs clearly say this is expected to work: Thread.join is described as "Wait until the thread terminates" and threading.enumerate's description says it will "Return a list of all currently active Thread objects ... It excludes terminated threads". So by the time t.join() returns, the thread should not be reported by threading.enumerate anymore. The bug appears to be that while the thread is shutting down, join() may exit too early: it waits for the __stopped flag to be set, but the thread hasn't necessarily been removed from the _active dict by that time, so enumerate will report a "stopped" thread. (Or if you like the bug is that __stopped is can be set while the thread is still in the _active dict.) A workaround is to filter the results of threading.enumerate() through [t for t in threading.enumerate() if t.isAlive()]. |
|
|
| msg31834 - (view) |
Author: Gabriel Genellina (ggenellina) |
Date: 2007-04-23 05:21 |
| The analysis appears to be correct, but I can't reproduce the bug with 2.5 on Windows XP SP2. |
|
|
| msg31835 - (view) |
Author: Gabriel Genellina (ggenellina) |
Date: 2007-04-23 05:51 |
| The analysis appears to be correct, but I can't reproduce the bug with 2.5 on Windows XP SP2. |
|
|
| msg31836 - (view) |
Author: Andrew Bennetts (spiv) |
Date: 2007-04-23 07:03 |
| gagenellina: thanks for taking a look! For what it's worth, I can reproduce this with that script using the Python 2.4 and 2.5 that come with Ubuntu 7.04 ("Feisty Fawn"), on an i386 laptop, so hopefully other people on a similar platform can also reproduce this. |
|
|
| msg61368 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2008-01-20 23:39 |
| I can reproduce this with both SVN trunk and 2.5.1 (on an x86 Mandriva box). |
|
|
| msg61369 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2008-01-21 00:26 |
| Also reproduced on winXP with SVN trunk. The cause seems that in threading.py, the __stop() method notifies all waiting threads, and that __delete() effectively removes the thread from the active list. I dared to swap the two calls, and it seems to solve the problem. Tests still pass, but can someone check that this can be done safely? |
|
|
| msg61460 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2008-01-21 22:42 |
| I'm not sure about the implications of Amaury's proposal, but in any case here is a patch which contains an unit test as well. (it seems to me swapping __stop() and __delete() can do no harm as there is no way for user code to be executed synchronously between both calls - unless you override one of these private methods) |
|
|
| msg61461 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2008-01-21 22:48 |
| Guido, you wrote that code... |
|
|
| msg61464 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2008-01-21 23:19 |
| Hm, this is multithreading. There is still the possibility of a switch between the two calls. In this case the thread is not marked as stopped, but enumerate() does not return it. Not easy to reproduce, though. |
|
|
| msg61466 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2008-01-21 23:34 |
| > There is still the possibility of a switch > between the two calls. In this case the thread is not marked as stopped, > but enumerate() does not return it. But user code is unlikely to rely on this because in most cases the thread *will* be marked as stopped ;) |
|
|
| msg61467 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2008-01-21 23:42 |
| Oops, sorry Amaury, I got your remark backwards. Nevermind... |
|
|
| msg61468 - (view) |
Author: Gregory P. Smith (gregory.p.smith) * |
Date: 2008-01-21 23:42 |
| why not just do this? finally: with _active_limbo_lock: self.__stop() try: self.__delete() except: pass (i believe with works on locks? if not turn that into an acquire, try: finally: release) |
|
|
| msg61469 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2008-01-22 00:05 |
| __delete() already acquires _active_limbo_lock so your proposal must be changed for the following: with _active_limbo_lock: self.__stop() try: del _active[_get_ident()] except: pass However, with this slight correction it seems to work. I was worrying whether another thread could hold __block (which is acquired in __stop()) while waiting for _active_limbo_lock to be released, but AFAICT there doesn't seem to be a code path where it can happen... |
|
|
| msg61473 - (view) |
Author: Gregory P. Smith (gregory.p.smith) * |
Date: 2008-01-22 01:29 |
| Looks good. Fixed in r60190 (2.6). And r60191 for release25-maint (2.5.2). |
|
|