classification
Title: "t.join(); assert t not in threading.enumerate()" fails
Type: Stage:
Components: Library (Lib) Versions: Python 2.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: gregory.p.smith Nosy List: amaury.forgeotdarc, georg.brandl, ggenellina, gregory.p.smith, gvanrossum, pitrou, rhettinger, spiv
Priority: normal Keywords: patch

Created on 2007-04-19 08:22 by spiv, last changed 2008-01-22 01:29 by gregory.p.smith. This issue is now closed.

Files
File name Uploaded Description Edit
thrbug.patch pitrou, 2008-01-21 22:42
thrbug2.patch pitrou, 2008-01-22 00:12 Gregory's more deterministic proposal
Messages (14)
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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2008-01-21 22:48
Guido, you wrote that code...
msg61464 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2008-01-21 23:42
Oops, sorry Amaury, I got your remark backwards. Nevermind...
msg61468 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2008-01-22 01:29
Looks good.  Fixed in r60190 (2.6).  And r60191 for release25-maint (2.5.2).
History
Date User Action Args
2008-01-22 01:29:21gregory.p.smithsetstatus: open -> closed
assignee: gvanrossum -> gregory.p.smith
messages: + msg61473
resolution: fixed
keywords: + patch
2008-01-22 00:12:19pitrousetfiles: + thrbug2.patch
2008-01-22 00:05:33pitrousetmessages: + msg61469
2008-01-21 23:42:50gregory.p.smithsetmessages: + msg61468
2008-01-21 23:42:05pitrousetmessages: + msg61467
2008-01-21 23:34:20pitrousetmessages: + msg61466
2008-01-21 23:19:32amaury.forgeotdarcsetmessages: + msg61464
2008-01-21 22:48:33georg.brandlsetassignee: rhettinger -> gvanrossum
messages: + msg61461
nosy: + gvanrossum, georg.brandl
2008-01-21 22:42:20pitrousetfiles: + thrbug.patch
messages: + msg61460
2008-01-21 00:26:15amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg61369
2008-01-20 23:39:50pitrousetnosy: + pitrou
messages: + msg61368
2008-01-17 01:42:07gregory.p.smithsetnosy: + gregory.p.smith
2007-04-19 08:22:36spivcreate