
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2011-04-18 11:29 byPeter.Saveliev, last changed2022-04-11 14:57 byadmin. This issue is nowclosed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| thread_test.py | Peter.Saveliev,2011-04-18 11:29 | simple test python script that illustrates _newname() race | ||
| newname_race_fix.patch | Peter.Saveliev,2011-05-03 08:05 | race condition fix (itertools, w/o locking) | ||
| Messages (10) | |||
|---|---|---|---|
| msg133963 -(view) | Author: Peter Saveliev (Peter.Saveliev) | Date: 2011-04-18 11:29 | |
The _newname() function has no locking.It is called from the new thread constructor. Such constructor is executed within parent thread. So, when several threads create new threads simultaneously, there can be race condition, leading to the same name for two (or even more) threads.8<-------------------------------->>> import threading>>> import dis>>> dis.dis(threading._newname)403 0 LOAD_GLOBAL 0 (_counter) 3 LOAD_CONST 1 (1) 6 BINARY_ADD 7 STORE_GLOBAL 0 (_counter)404 10 LOAD_FAST 0 (template) 13 LOAD_GLOBAL 0 (_counter) 16 BINARY_MODULO 17 RETURN_VALUE >>> 8<--------------------------------The race condition can be achieved between BINARY_ADD and STORE_GLOBAL. Several threads can do BINARY_ADD before the first one will do STORE_GLOBAL. All racing threads will get the same name.8<--------------------------------$ for i in `seq 0 100`; do python thread_test.py |\ awk -F Thread- '{print $2}' |\ grep -vE '^$' |\ sort |\ uniq -c -d; done 2 35 2 12 ...8<--------------------------------As you see, there are cases when several threads can get same name.Proposals: use thread-safe increment counter (with atomic get-increment) like itertools.counter() (that does not release GIL) | |||
| msg135004 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2011-05-02 19:59 | |
Do you want to provide a patch? | |||
| msg135019 -(view) | Author: Peter Saveliev (Peter.Saveliev) | Date: 2011-05-03 08:05 | |
Ok, patch attached.Patch made for Python: 2.6Tested Python versions: 2.6, 2.7 | |||
| msg141990 -(view) | Author: Peter Saveliev (Peter.Saveliev) | Date: 2011-08-12 21:00 | |
Any news? I hope, the change is trivial enough… | |||
| msg141994 -(view) | Author: Amaury Forgeot d'Arc (amaury.forgeotdarc)*![]() | Date: 2011-08-12 22:52 | |
Are you sure that counter.next() cannot release the GIL? Remember that any DECREF can trigger the garbage collector and execute arbitrary code... | |||
| msg142015 -(view) | Author: Peter Saveliev (Peter.Saveliev) | Date: 2011-08-13 07:01 | |
counter.next() is a C routine and it is atomic from Python's point of view — if I understand right.The test shows that original threading.py leads to a (rare) race here, while with counter object there is no race condition. | |||
| msg142016 -(view) | Author: Raymond Hettinger (rhettinger)*![]() | Date: 2011-08-13 07:52 | |
I think the patch is correct.FWIW, my style is to prebind the next method, making the counter completely self-contained (like a closure):+_counter = itertools.count().next def _newname(template="Thread-%d"): global _counter- _counter = _counter + 1- return template % _counter+ return template % _counter() | |||
| msg169656 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2012-09-01 18:06 | |
With or without Raymond's suggested change, "global _counter" is not necessary anymore.Note that this fix only works on implementations where itertools.count().next is atomic. | |||
| msg228498 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2014-10-04 21:47 | |
New changeseta6b4add67168 by R David Murray in branch '2.7':#11866: Eliminate race condition in the computation of names for new threads.https://hg.python.org/cpython/rev/a6b4add67168New changeseta6906b9e21d5 by R David Murray in branch '3.4':#11866: Eliminate race condition in the computation of names for new threads.https://hg.python.org/cpython/rev/a6906b9e21d5New changesete9afcce9a154 by R David Murray in branch 'default':Merge:#11866: Eliminate race condition in the computation of names for new threads.https://hg.python.org/cpython/rev/e9afcce9a154 | |||
| msg228499 -(view) | Author: R. David Murray (r.david.murray)*![]() | Date: 2014-10-04 21:50 | |
I committed a version of this that uses Raymond's suggestion and also makes sure that the first thread is named Thread-1 as it has been in the past (there was a test failure without that fix, but I also think it makes sense for it to be Thread-1 since one would normally think of the main thread as thread 0).I did not try to turn the test script into a test case because it would take a long time to run if it is to have a reasonable chance of displaying the bug, and it would never be a consistent failure.Thanks, Peter; sorry it took so long to get this committed. | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:57:16 | admin | set | github: 56075 |
| 2014-10-04 21:50:27 | r.david.murray | set | status: open -> closed versions: + Python 3.4, Python 3.5, - Python 2.6, Python 3.1, Python 3.2, Python 3.3 nosy: +r.david.murray messages: +msg228499 resolution: accepted -> fixed stage: commit review -> resolved |
| 2014-10-04 21:47:10 | python-dev | set | nosy: +python-dev messages: +msg228498 |
| 2012-09-01 18:06:30 | pitrou | set | messages: +msg169656 |
| 2012-09-01 17:58:49 | neologix | set | stage: commit review |
| 2011-08-13 07:52:09 | rhettinger | set | nosy: +rhettinger messages: +msg142016 assignee:rhettinger resolution: accepted |
| 2011-08-13 07:01:36 | Peter.Saveliev | set | messages: +msg142015 |
| 2011-08-12 22:52:14 | amaury.forgeotdarc | set | nosy: +amaury.forgeotdarc messages: +msg141994 |
| 2011-08-12 21:00:24 | Peter.Saveliev | set | messages: +msg141990 |
| 2011-05-03 08:05:51 | Peter.Saveliev | set | files: +newname_race_fix.patch keywords: +patch messages: +msg135019 versions: + Python 2.6 |
| 2011-05-02 19:59:34 | pitrou | set | versions: + Python 3.1, Python 2.7, Python 3.2, Python 3.3, - Python 2.6 nosy: +pitrou messages: +msg135004 components: + Library (Lib), - Extension Modules |
| 2011-04-18 11:29:02 | Peter.Saveliev | create | |