diff options
author | Gregory P. Smith <greg@mad-scientist.com> | 2008-08-04 07:33:37 (GMT) |
---|---|---|
committer | Gregory P. Smith <greg@mad-scientist.com> | 2008-08-04 07:33:37 (GMT) |
commit | d868be8805b7f5c4f0356eb01f6d8feab43cc9b3 (patch) | |
tree | 7a85f9173c432c9146ff2629eac9557edec6693b | |
parent | e7829a5b1b75fdee4f9b7afebeceb98be2d3b0d7 (diff) | |
download | cpython-d868be8805b7f5c4f0356eb01f6d8feab43cc9b3.zip cpython-d868be8805b7f5c4f0356eb01f6d8feab43cc9b3.tar.gz cpython-d868be8805b7f5c4f0356eb01f6d8feab43cc9b3.tar.bz2 |
Adds a sanity check to avoid a *very rare* infinite loop due to a corrupt tls
key list data structure in the thread startup path.
This change is a companion to r60148 which already successfully dealt with a
similar issue on thread shutdown.
In particular this loop has been observed happening from this call path:
#0 in find_key ()
#1 in PyThread_set_key_value ()
#2 in _PyGILState_NoteThreadState ()
#3 in PyThreadState_New ()
#4 in t_bootstrap ()
#5 in pthread_start_thread ()
I don't know how this happens but it does, *very* rarely. On more than
one hardware platform. I have not been able to reproduce it manually.
(A flaky mutex implementation on the system in question is one hypothesis).
As with r60148, the spinning we managed to observe in the wild was due to a
single list element pointing back upon itself.
-rw-r--r-- | Python/pystate.c | 4 | ||||
-rw-r--r-- | Python/thread.c | 12 |
2 files changed, 15 insertions, 1 deletions
diff --git a/Python/pystate.c b/Python/pystate.c index 36b06d6..da417c1 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -253,6 +253,10 @@ tstate_delete_common(PyThreadState *tstate) "PyThreadState_Delete: invalid tstate"); if (*p == tstate) break; + /* Sanity check. These states should never happen but if + * they do we must abort. Otherwise we'll end up spinning in + * in a tight loop with the lock held. A similar check is done + * in thread.c find_key(). */ if (*p == prev_p) Py_FatalError( "PyThreadState_Delete: small circular list(!)" diff --git a/Python/thread.c b/Python/thread.c index b21e53b..03580f3 100644 --- a/Python/thread.c +++ b/Python/thread.c @@ -264,15 +264,25 @@ static int nkeys = 0; /* PyThread_create_key() hands out nkeys+1 next */ static struct key * find_key(int key, void *value) { - struct key *p; + struct key *p, *prev_p; long id = PyThread_get_thread_ident(); if (!keymutex) return NULL; PyThread_acquire_lock(keymutex, 1); + prev_p = NULL; for (p = keyhead; p != NULL; p = p->next) { if (p->id == id && p->key == key) goto Done; + /* Sanity check. These states should never happen but if + * they do we must abort. Otherwise we'll end up spinning in + * in a tight loop with the lock held. A similar check is done + * in pystate.c tstate_delete_common(). */ + if (p == prev_p) + Py_FatalError("tls find_key: small circular list(!)"); + prev_p = p; + if (p->next == keyhead) + Py_FatalError("tls find_key: circular list(!)"); } if (value == NULL) { assert(p == NULL); |