| msg57225 - (view) |
Author: Ronald Oussoren (ronaldoussoren) *  |
Date: 2007-11-07 22:05 |
| Py_Finalize cleans up the thread state by first calling _PyGILState_Fini and then calling PyInterpreterState_Clear. The latter can cause user code to run, which could use the GILState API and this then causes a crash. The attached file 'threads.py' causes a crash on OSX leopard because of this issue. The script causes an exception to be set that has an attribute that uses the GILState API in its dealloc slot. |
|
|
| msg57285 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-09 00:05 |
| Do you have a patch? Then we could consider fixing this in 2.5.2. |
|
|
| msg57297 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2007-11-09 05:23 |
| I don't have a patch. I don't even know enough of the threading infrastructure to know if this really a bug or if it is a bug in my code. I'll work on a patch this weekend, if changing the order of calls to PyGILState_Fini and PyInterpreterState_Clear doesn't break unittests and fixes the problem I ran into I'll post about this issue on python-dev. |
|
|
| msg57754 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2007-11-22 10:52 |
| The attached patch fixes the crash, but I haven't verified if the patch is actually correct. With this patch some PyThread API's are called after PyInterpreterState_Clear and I don't know if it is valid to do so. All unittests pass fine on OSX though. |
|
|
| msg57795 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2007-11-23 23:34 |
| I managed to reproduce the problem consistently with the following code: import ctypes, sys, time, thread # Module globals are cleared before __del__ is run # So save the functions in class dict class C: ensure = ctypes.pythonapi.PyGILState_Ensure release = ctypes.pythonapi.PyGILState_Release def __del__(self): state = self.ensure() self.release(state) def waitingThread(): x = C() time.sleep(100) thread.start_new_thread(waitingThread, ()) time.sleep(1) sys.exit(42) On exit, PyInterpreterState_Clear stops the sleeping thread and free its local variables. But at this time _PyGILState_Fini has already been called... The initial patch also corrects this problem. I join another patch, which adds the above code in a testcase. Ready to check-in, but can someone have a look? |
|
|
| msg57856 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-26 22:03 |
| So is this a Mac-only issue? And couldn't the GIL state cleanup also invoke user code, which might be abused to create more threads, wreaking havoc that way? I'm kind of worried about putting this into 2.5.2 and breaking somebody's working code (that's always worse than fixing somebody's broken code... :-). |
|
|
| msg57865 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2007-11-27 05:38 |
| This is not a mac-specific issue, the script happens to be mac-specific because that's how I found the issue. Amaury's patch adds a unittest that reproduces the problem in a platform-indepenent way using ctypes. The _PyGILState_Fini function might cause user code to run as well, it removes the thread-local variable that contains the PyThreadState for threads, and that contains some Python objects that might contain arbitrary values (such as the last exception value). I guess that means that a variation on Amaury's patch would cause a patched interpreter to crash (create on instance of 'C' as an attribute on an exception, e.g. raise ValueError(C()) in the second thread). BTW. I would be very uncomfortable if my patch were applied without review from someone that knows his way around the threading code. BTW2. I have a workaround for my initial issue (the crash in threads.py): if I add an atexit handler that performs some cleanup in PyObjC the problem goes away. |
|
|
| msg57866 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2007-11-27 08:20 |
| > The _PyGILState_Fini function might cause user code to run as well, > it removes the thread-local variable that contains the PyThreadState > for threads, and that contains some Python objects that might contain > arbitrary values (such as the last exception value). No, _PyGILState_Fini does not invoke any python code; it only clears C variables. The last exception value is deleted during the call to PyInterpreterState_Clear() (inside PyThreadState_Clear). |
|
|
| msg57877 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-27 18:39 |
| > No, _PyGILState_Fini does not invoke any python code You're right. It calls PyThread_delete_key(). I thought this would delete entries from a dictionary (thereby potentially invoking Python code via Py_DECREF()), but it doesn't -- it just frees some memory. So I'm okay with swapping the order in 2.6 and 3.0. I'm still not 100% comfortable with doing this in 2.5, especially since Ronald found a work-around for his immediate issue. |
|
|
| msg57966 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2007-11-29 23:36 |
| Committed revision 59231 in trunk. |
|
|