Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GIL destruction can fail #54110

Closed
pitrou opened this issue Sep 19, 2010 · 5 comments
Closed

GIL destruction can fail #54110

pitrou opened this issue Sep 19, 2010 · 5 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@pitrou
Copy link
Member

pitrou commented Sep 19, 2010

BPO 9901
Nosy @amauryfa, @pitrou

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2010-09-20.22:53:51.804>
created_at = <Date 2010-09-19.23:57:40.339>
labels = ['interpreter-core', 'type-bug']
title = 'GIL destruction can fail'
updated_at = <Date 2010-09-20.22:53:51.803>
user = 'https://github.com/pitrou'

bugs.python.org fields:

activity = <Date 2010-09-20.22:53:51.803>
actor = 'pitrou'
assignee = 'none'
closed = True
closed_date = <Date 2010-09-20.22:53:51.804>
closer = 'pitrou'
components = ['Interpreter Core']
creation = <Date 2010-09-19.23:57:40.339>
creator = 'pitrou'
dependencies = []
files = []
hgrepos = []
issue_num = 9901
keywords = []
message_count = 5.0
messages = ['116893', '116913', '116925', '116926', '116968']
nosy_count = 2.0
nosy_names = ['amaury.forgeotdarc', 'pitrou']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue9901'
versions = ['Python 3.2']

@pitrou
Copy link
Member Author

pitrou commented Sep 19, 2010

test_finalize_with_trace (in test_threading) sometimes fails because of failing to destroy the GIL (in _PyEval_FiniThreads()). This can be reproduced quite reliably by launching several copies in parallel:

$ ./python -m test.regrtest -j10 -F test_threading
[...]
test test_threading failed -- Traceback (most recent call last):
  File "/home/antoine/py3k/__svn__/Lib/test/test_threading.py", line 334, in test_finalize_with_trace
    "Unexpected error: " + ascii(stderr))
AssertionError: Unexpected error: b'Fatal Python error: pthread_mutex_destroy(gil_mutex) failed\n'

What happens is that pthread_mutex_destroy() fails with EBUSY. According to the POSIX man page:

“[EBUSY]
The implementation has detected an attempt to destroy the object referenced by mutex while it is locked or referenced (for example, while being used in a pthread_cond_timedwait() or pthread_cond_wait()) by another thread.”

After a bit of tracing, it becomes clear that Py_Finalize() calls _PyEval_FiniThreads() while another thread is taking the GIL (take_gil()). Unfortunately, this is not a situation we can avoid, since we rely on process exit to kill lingering threads: arbitrary CPython code may still be running in parallel while we are finalizing interpreter structures.

Therefore, it is likely that _PyEval_FiniThreads() should avoid destroying the mutex at all. Indeed, if we destroy the mutex, it is possible that a lingering thread tries to retake the GIL after waking up from a system call (Py_END_ALLOW_THREADS), and fails because of another fatal error ("Fatal Python error: pthread_mutex_lock(gil_mutex) failed").

@pitrou pitrou added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Sep 19, 2010
@amauryfa
Copy link
Member

looks similar to bpo-1856

@pitrou
Copy link
Member Author

pitrou commented Sep 20, 2010

It is similar, but the issue is slightly different. Fixing bpo-1856 with the proposed resolution (the "stay_forever" flag) won't solve this, because the GIL mutex will still refuse to be destroyed if other threads reference it at the same time.

@pitrou
Copy link
Member Author

pitrou commented Sep 20, 2010

Moving the _PyEval_FiniThreads() call to Py_Initialize() solves the issue:

diff -r 9e49082da463 Python/pythonrun.c
--- a/Python/pythonrun.c        Mon Sep 20 12:46:56 2010 +0200
+++ b/Python/pythonrun.c        Mon Sep 20 13:41:47 2010 +0200
@@ -217,8 +217,15 @@ Py_InitializeEx(int install_sigs)
         Py_FatalError("Py_Initialize: can't make first thread");
     (void) PyThreadState_Swap(tstate);
 
-    /* auto-thread-state API, if available */
 #ifdef WITH_THREAD
+    /* We can't call _PyEval_FiniThreads() in Py_Finalize because
+       destroying the GIL might fail when it is being referenced from
+       another running thread (see issue python/cpython#54110).
+       Instead we destroy the previously created GIL here, which ensures
+       that we can call Py_Initialize / Py_Finalize multiple times. */
+    _PyEval_FiniThreads();
+
+    /* Auto-thread-state API */
     _PyGILState_Init(interp, tstate);
 #endif /* WITH_THREAD */
 
@@ -514,10 +521,6 @@ Py_Finalize(void)
 
     PyGrammar_RemoveAccelerators(&_PyParser_Grammar);
 
-#ifdef WITH_THREAD
-    _PyEval_FiniThreads();
-#endif
-
 #ifdef Py_TRACE_REFS
     /* Display addresses (& refcnts) of all objects still alive.
      * An address can be used to find the repr of the object, printed

@pitrou
Copy link
Member Author

pitrou commented Sep 20, 2010

Committed in r84927.

@pitrou pitrou closed this as completed Sep 20, 2010
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants