From 441adb8c5782de6fcf71ee3396d114c48cd1cb5b Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 13 Dec 2013 04:14:41 +0100 Subject: [PATCH] Backout changeset 46393019b650 test_capi is failing and the fix is not trivial, I prefer to revert --- Doc/whatsnew/3.4.rst | 5 ----- Misc/NEWS | 4 ---- Modules/_testcapimodule.c | 4 ---- Modules/_tracemalloc.c | 9 ++++++--- Python/thread.c | 20 ++++++++++++-------- Python/thread_nt.h | 9 +++++++++ Python/thread_pthread.h | 3 +++ 7 files changed, 30 insertions(+), 24 deletions(-) diff --git a/Doc/whatsnew/3.4.rst b/Doc/whatsnew/3.4.rst index 8fe906be218..5d360c4d515 100644 --- a/Doc/whatsnew/3.4.rst +++ b/Doc/whatsnew/3.4.rst @@ -1068,8 +1068,3 @@ that may require changes to your code. working directory will also now have an absolute path, including when using ``-m`` with the interpreter (this does not influence when the path to a file is specified on the command-line). - -* (C API) :c:func:`PyThread_set_key_value` now always set the value. In Python - 3.3, the function did nothing if the key already exists (if the current - value is a non-NULL pointer). - diff --git a/Misc/NEWS b/Misc/NEWS index 800002a89d9..8be054f7657 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,10 +10,6 @@ Release date: 2014-01-05 Core and Builtins ----------------- -- Issue #19787: PyThread_set_key_value() now always set the value. In Python - 3.3, the function did nothing if the key already exists (if the current value - is a non-NULL pointer). - - Issue #14432: Remove the thread state field from the frame structure. Fix a crash when a generator is created in a C thread that is destroyed while the generator is still used. The issue was that a generator contains a frame, and diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 6f2a75cdad6..a0cffde5bfa 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2511,10 +2511,6 @@ run_in_subinterp(PyObject *self, PyObject *args) r = PyRun_SimpleString(code); Py_EndInterpreter(substate); - /* restore previous thread safe. It was replaced by Py_NewInterpreter() - which creates a new thread state. */ - _PyThreadState_Init(mainstate); - PyThreadState_Swap(mainstate); return PyLong_FromLong(r); diff --git a/Modules/_tracemalloc.c b/Modules/_tracemalloc.c index 95b05d61c55..b39e950c273 100644 --- a/Modules/_tracemalloc.c +++ b/Modules/_tracemalloc.c @@ -168,11 +168,14 @@ set_reentrant(int reentrant) assert(reentrant == 0 || reentrant == 1); if (reentrant) { assert(PyThread_get_key_value(tracemalloc_reentrant_key) == NULL); - PyThread_set_key_value(tracemalloc_reentrant_key, REENTRANT); + PyThread_set_key_value(tracemalloc_reentrant_key, + REENTRANT); } else { - assert(PyThread_get_key_value(tracemalloc_reentrant_key) == REENTRANT); - PyThread_set_key_value(tracemalloc_reentrant_key, NULL); + /* FIXME: PyThread_set_key_value() cannot be used to set the flag + to zero, because it does nothing if the variable has already + a value set. */ + PyThread_delete_key_value(tracemalloc_reentrant_key); } } diff --git a/Python/thread.c b/Python/thread.c index 5396ca30e3e..8540942e28a 100644 --- a/Python/thread.c +++ b/Python/thread.c @@ -205,7 +205,7 @@ static int nkeys = 0; /* PyThread_create_key() hands out nkeys+1 next */ * segfaults. Now we lock the whole routine. */ static struct key * -find_key(int key, int update, void *value) +find_key(int key, void *value) { struct key *p, *prev_p; long id = PyThread_get_thread_ident(); @@ -215,11 +215,8 @@ find_key(int key, int update, void *value) PyThread_acquire_lock(keymutex, 1); prev_p = NULL; for (p = keyhead; p != NULL; p = p->next) { - if (p->id == id && p->key == key) { - if (update) - p->value = value; + 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 @@ -230,7 +227,7 @@ find_key(int key, int update, void *value) if (p->next == keyhead) Py_FatalError("tls find_key: circular list(!)"); } - if (!update && value == NULL) { + if (value == NULL) { assert(p == NULL); goto Done; } @@ -282,12 +279,19 @@ PyThread_delete_key(int key) PyThread_release_lock(keymutex); } +/* Confusing: If the current thread has an association for key, + * value is ignored, and 0 is returned. Else an attempt is made to create + * an association of key to value for the current thread. 0 is returned + * if that succeeds, but -1 is returned if there's not enough memory + * to create the association. value must not be NULL. + */ int PyThread_set_key_value(int key, void *value) { struct key *p; - p = find_key(key, 1, value); + assert(value != NULL); + p = find_key(key, value); if (p == NULL) return -1; else @@ -300,7 +304,7 @@ PyThread_set_key_value(int key, void *value) void * PyThread_get_key_value(int key) { - struct key *p = find_key(key, 0, NULL); + struct key *p = find_key(key, NULL); if (p == NULL) return NULL; diff --git a/Python/thread_nt.h b/Python/thread_nt.h index ee2079fc613..ab5a08168f7 100644 --- a/Python/thread_nt.h +++ b/Python/thread_nt.h @@ -389,11 +389,20 @@ PyThread_delete_key(int key) TlsFree(key); } +/* We must be careful to emulate the strange semantics implemented in thread.c, + * where the value is only set if it hasn't been set before. + */ int PyThread_set_key_value(int key, void *value) { BOOL ok; + void *oldvalue; + assert(value != NULL); + oldvalue = TlsGetValue(key); + if (oldvalue != NULL) + /* ignore value if already set */ + return 0; ok = TlsSetValue(key, value); if (!ok) return -1; diff --git a/Python/thread_pthread.h b/Python/thread_pthread.h index d9f7c76f2ab..20f85358968 100644 --- a/Python/thread_pthread.h +++ b/Python/thread_pthread.h @@ -627,6 +627,9 @@ int PyThread_set_key_value(int key, void *value) { int fail; + void *oldValue = pthread_getspecific(key); + if (oldValue != NULL) + return 0; fail = pthread_setspecific(key, value); return fail ? -1 : 0; }