From 9ce55538224615160df3298bbb0c6b324119132b Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Thu, 8 Jun 2017 14:37:32 -0700 Subject: [PATCH 01/13] Move co_extra_freefuncs to interpreter state to avoid crashes in multi-threaded scenarios involving deletion of code objects --- Include/pystate.h | 6 +-- Lib/test/test_code.py | 103 +++++++++++++++++++++++++++++++++++++++++- Objects/codeobject.c | 25 ++++++---- Python/ceval.c | 8 ++-- Python/pystate.c | 2 +- 5 files changed, 125 insertions(+), 19 deletions(-) diff --git a/Include/pystate.h b/Include/pystate.h index afc3c0c6d1d5ac..226017f07be72a 100644 --- a/Include/pystate.h +++ b/Include/pystate.h @@ -48,6 +48,9 @@ typedef struct _is { PyObject *import_func; /* Initialized to PyEval_EvalFrameDefault(). */ _PyFrameEvalFunction eval_frame; + + Py_ssize_t co_extra_user_count; + freefunc co_extra_freefuncs[MAX_CO_EXTRA_USERS]; } PyInterpreterState; #endif @@ -142,9 +145,6 @@ typedef struct _ts { PyObject *coroutine_wrapper; int in_coroutine_wrapper; - Py_ssize_t co_extra_user_count; - freefunc co_extra_freefuncs[MAX_CO_EXTRA_USERS]; - PyObject *async_gen_firstiter; PyObject *async_gen_finalizer; diff --git a/Lib/test/test_code.py b/Lib/test/test_code.py index 7975ea0ef5e17d..74d3af309833a0 100644 --- a/Lib/test/test_code.py +++ b/Lib/test/test_code.py @@ -105,7 +105,9 @@ import sys import unittest import weakref -from test.support import run_doctest, run_unittest, cpython_only +import threading +from test.support import (run_doctest, run_unittest, cpython_only, + check_impl_detail) def consts(t): @@ -212,10 +214,107 @@ def callback(code): self.assertTrue(self.called) +if check_impl_detail(cpython=True): + import ctypes + py = ctypes.pythonapi + freefunc = ctypes.CFUNCTYPE(None,ctypes.c_voidp) + + RequestCodeExtraIndex = py._PyEval_RequestCodeExtraIndex + RequestCodeExtraIndex.argtypes = (freefunc, ) + RequestCodeExtraIndex.restype = ctypes.c_ssize_t + + SetExtra = py._PyCode_SetExtra + SetExtra.argtypes = (ctypes.py_object, ctypes.c_ssize_t, ctypes.c_voidp) + SetExtra.restype = ctypes.c_int + + GetExtra = py._PyCode_GetExtra + GetExtra.argtypes = (ctypes.py_object, ctypes.c_ssize_t, + ctypes.POINTER(ctypes.c_voidp)) + GetExtra.restype = ctypes.c_int + + LAST_FREED = None + def myfree(ptr): + global LAST_FREED + LAST_FREED = ptr + + FREE_FUNC = freefunc(myfree) + FREE_INDEX = RequestCodeExtraIndex(FREE_FUNC) + + class CoExtra(unittest.TestCase): + def get_func(self): + # Defining a function causes the containing function to have a + # reference to the code object. We need the code objects to go + # away, so we eval a lambda. + return eval('lambda:42') + + def test_get_non_code(self): + f = self.get_func() + + self.assertRaises(SystemError, SetExtra, 42, FREE_INDEX, + ctypes.c_voidp(100)) + self.assertRaises(SystemError, GetExtra, 42, FREE_INDEX, + ctypes.c_voidp(100)) + + def test_bad_index(self): + f = self.get_func() + self.assertRaises(SystemError, SetExtra, f.__code__, + FREE_INDEX+100, ctypes.c_voidp(100)) + self.assertEqual(GetExtra(f.__code__, FREE_INDEX+100, + ctypes.c_voidp(100)), 0) + + def test_free_called(self): + '''Verify that the provided free function gets invoked + when the code object is cleaned up''' + f = self.get_func() + + SetExtra(f.__code__, FREE_INDEX, ctypes.c_voidp(100)) + del f + self.assertEqual(LAST_FREED, 100) + + def test_get_set(self): + '''Basic get/set round tripping''' + f = self.get_func() + + extra = ctypes.c_voidp() + + SetExtra(f.__code__, FREE_INDEX, ctypes.c_voidp(200)) + # reset should free... + SetExtra(f.__code__, FREE_INDEX, ctypes.c_voidp(300)) + self.assertEqual(LAST_FREED, 200) + + extra = ctypes.c_voidp() + GetExtra(f.__code__, FREE_INDEX, extra) + self.assertEqual(extra.value, 300) + del f + + def test_free_different_thread(self): + '''Freeing a code object on a different thread then + where the co_extra was set should be safe''' + f = self.get_func() + class ThreadTest(threading.Thread): + def __init__(self, f, test): + super().__init__() + self.f = f + self.test = test + def run(self): + del self.f + self.test.assertEqual(LAST_FREED, 500) + + SetExtra(f.__code__, FREE_INDEX, ctypes.c_voidp(500)) + tt = ThreadTest(f, self) + del f + tt.start() + tt.join() + self.assertEqual(LAST_FREED, 500) + +else: + class CoExtra(unittest.TestCase): + pass + def test_main(verbose=None): from test import test_code run_doctest(test_code, verbose) - run_unittest(CodeTest, CodeConstsTest, CodeWeakRefTest) + run_unittest(CodeTest, CodeConstsTest, CodeWeakRefTest, CoExtra) if __name__ == "__main__": diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 22c4f856cd83e6..f5d3b21e8d3b51 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -411,11 +411,11 @@ static void code_dealloc(PyCodeObject *co) { if (co->co_extra != NULL) { - PyThreadState *tstate = PyThreadState_Get(); + PyInterpreterState *interp = PyThreadState_Get()->interp; _PyCodeObjectExtra *co_extra = co->co_extra; for (Py_ssize_t i = 0; i < co_extra->ce_size; i++) { - freefunc free_extra = tstate->co_extra_freefuncs[i]; + freefunc free_extra = interp->co_extra_freefuncs[i]; if (free_extra != NULL) { free_extra(co_extra->ce_extras[i]); @@ -848,10 +848,10 @@ _PyCode_GetExtra(PyObject *code, Py_ssize_t index, void **extra) int _PyCode_SetExtra(PyObject *code, Py_ssize_t index, void *extra) { - PyThreadState *tstate = PyThreadState_Get(); + PyInterpreterState *interp = PyThreadState_Get()->interp; if (!PyCode_Check(code) || index < 0 || - index >= tstate->co_extra_user_count) { + index >= interp->co_extra_user_count) { PyErr_BadInternalCall(); return -1; } @@ -866,13 +866,13 @@ _PyCode_SetExtra(PyObject *code, Py_ssize_t index, void *extra) } co_extra->ce_extras = PyMem_Malloc( - tstate->co_extra_user_count * sizeof(void*)); + interp->co_extra_user_count * sizeof(void*)); if (co_extra->ce_extras == NULL) { PyMem_Free(co_extra); return -1; } - co_extra->ce_size = tstate->co_extra_user_count; + co_extra->ce_size = interp->co_extra_user_count; for (Py_ssize_t i = 0; i < co_extra->ce_size; i++) { co_extra->ce_extras[i] = NULL; @@ -882,20 +882,27 @@ _PyCode_SetExtra(PyObject *code, Py_ssize_t index, void *extra) } else if (co_extra->ce_size <= index) { void** ce_extras = PyMem_Realloc( - co_extra->ce_extras, tstate->co_extra_user_count * sizeof(void*)); + co_extra->ce_extras, interp->co_extra_user_count * sizeof(void*)); if (ce_extras == NULL) { return -1; } for (Py_ssize_t i = co_extra->ce_size; - i < tstate->co_extra_user_count; + i < interp->co_extra_user_count; i++) { ce_extras[i] = NULL; } co_extra->ce_extras = ce_extras; - co_extra->ce_size = tstate->co_extra_user_count; + co_extra->ce_size = interp->co_extra_user_count; + } + + if (co_extra->ce_extras[index] != NULL) { + freefunc free = interp->co_extra_freefuncs[index]; + if (free != NULL) { + free(co_extra->ce_extras[index]); + } } co_extra->ce_extras[index] = extra; diff --git a/Python/ceval.c b/Python/ceval.c index 5dc0444a1acf5c..37fb2fc2b25c45 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -5442,14 +5442,14 @@ _Py_GetDXProfile(PyObject *self, PyObject *args) Py_ssize_t _PyEval_RequestCodeExtraIndex(freefunc free) { - PyThreadState *tstate = PyThreadState_Get(); + PyInterpreterState *interp = PyThreadState_Get()->interp; Py_ssize_t new_index; - if (tstate->co_extra_user_count == MAX_CO_EXTRA_USERS - 1) { + if (interp->co_extra_user_count == MAX_CO_EXTRA_USERS - 1) { return -1; } - new_index = tstate->co_extra_user_count++; - tstate->co_extra_freefuncs[new_index] = free; + new_index = interp->co_extra_user_count++; + interp->co_extra_freefuncs[new_index] = free; return new_index; } diff --git a/Python/pystate.c b/Python/pystate.c index ccb0092c42b3a4..9f57e8051eb1d6 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -92,6 +92,7 @@ PyInterpreterState_New(void) interp->importlib = NULL; interp->import_func = NULL; interp->eval_frame = _PyEval_EvalFrameDefault; + interp->co_extra_user_count = 0; #ifdef HAVE_DLOPEN #if HAVE_DECL_RTLD_NOW interp->dlopenflags = RTLD_NOW; @@ -224,7 +225,6 @@ new_threadstate(PyInterpreterState *interp, int init) tstate->coroutine_wrapper = NULL; tstate->in_coroutine_wrapper = 0; - tstate->co_extra_user_count = 0; tstate->async_gen_firstiter = NULL; tstate->async_gen_finalizer = NULL; From b976f1b6702687f53248e8cdb001f2b1f7d5b17a Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Thu, 8 Jun 2017 15:19:14 -0700 Subject: [PATCH 02/13] Don't require that extra be zero initialized --- Objects/codeobject.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/Objects/codeobject.c b/Objects/codeobject.c index f5d3b21e8d3b51..c01e7562d034e8 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -825,8 +825,6 @@ _PyCode_CheckLineNumber(PyCodeObject* co, int lasti, PyAddrPair *bounds) int _PyCode_GetExtra(PyObject *code, Py_ssize_t index, void **extra) { - assert(*extra == NULL); - if (!PyCode_Check(code)) { PyErr_BadInternalCall(); return -1; From 642e5831681ea7cc56625cd370ab2a7dc673f9bf Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Thu, 8 Jun 2017 16:01:21 -0700 Subject: [PATCH 03/13] Build test list instead of defining empty test class --- Lib/test/test_code.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_code.py b/Lib/test/test_code.py index 74d3af309833a0..b6965dc27a98df 100644 --- a/Lib/test/test_code.py +++ b/Lib/test/test_code.py @@ -307,15 +307,13 @@ def run(self): tt.join() self.assertEqual(LAST_FREED, 500) -else: - class CoExtra(unittest.TestCase): - pass - def test_main(verbose=None): from test import test_code run_doctest(test_code, verbose) - run_unittest(CodeTest, CodeConstsTest, CodeWeakRefTest, CoExtra) - + tests = [CodeTest, CodeConstsTest, CodeWeakRefTest] + if check_impl_detail(cpython=True): + tests.append(CoExtra) + run_unittest(*tests) if __name__ == "__main__": test_main() From 93a68dd8d6c5bae4564cf079354430e657e1557f Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Thu, 8 Jun 2017 16:03:38 -0700 Subject: [PATCH 04/13] Ensure extra is always assigned on success --- Objects/codeobject.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Objects/codeobject.c b/Objects/codeobject.c index c01e7562d034e8..d091fa286e95da 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -835,6 +835,7 @@ _PyCode_GetExtra(PyObject *code, Py_ssize_t index, void **extra) if (co_extra == NULL || co_extra->ce_size <= index) { + *extra = NULL; return 0; } From a33106bda5bcf54e5209717a53739c6bea201743 Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Fri, 9 Jun 2017 16:00:00 -0700 Subject: [PATCH 05/13] Keep the old fields in the thread state object, just don't use them Add new linked list of code extra objects on a per-interpreter basis so that interpreter state size isn't changed --- Include/pystate.h | 15 +++++++++++++-- Objects/codeobject.c | 20 ++++++++++---------- Python/ceval.c | 8 ++++---- Python/pystate.c | 44 ++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 69 insertions(+), 18 deletions(-) diff --git a/Include/pystate.h b/Include/pystate.h index 226017f07be72a..8f6f5e2725f018 100644 --- a/Include/pystate.h +++ b/Include/pystate.h @@ -48,12 +48,18 @@ typedef struct _is { PyObject *import_func; /* Initialized to PyEval_EvalFrameDefault(). */ _PyFrameEvalFunction eval_frame; +} PyInterpreterState; +#endif + +typedef struct _co_extra_state { + struct _co_extra_state *next; + PyInterpreterState* interp; Py_ssize_t co_extra_user_count; freefunc co_extra_freefuncs[MAX_CO_EXTRA_USERS]; -} PyInterpreterState; -#endif +} PyCodeExtraState; +PyCodeExtraState* _PyCodeExtraState_Get(); /* State unique per thread */ @@ -145,6 +151,11 @@ typedef struct _ts { PyObject *coroutine_wrapper; int in_coroutine_wrapper; + /* Now used from PyInterpreterState, kept here for ABI + compatibility with PyThreadState */ + Py_ssize_t _preserve_36_ABI_1; + freefunc _preserve_36_ABI_2[MAX_CO_EXTRA_USERS]; + PyObject *async_gen_firstiter; PyObject *async_gen_finalizer; diff --git a/Objects/codeobject.c b/Objects/codeobject.c index d091fa286e95da..6245747fa45616 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -411,11 +411,11 @@ static void code_dealloc(PyCodeObject *co) { if (co->co_extra != NULL) { - PyInterpreterState *interp = PyThreadState_Get()->interp; + PyCodeExtraState *state = _PyCodeExtraState_Get(); _PyCodeObjectExtra *co_extra = co->co_extra; for (Py_ssize_t i = 0; i < co_extra->ce_size; i++) { - freefunc free_extra = interp->co_extra_freefuncs[i]; + freefunc free_extra = state->co_extra_freefuncs[i]; if (free_extra != NULL) { free_extra(co_extra->ce_extras[i]); @@ -847,10 +847,10 @@ _PyCode_GetExtra(PyObject *code, Py_ssize_t index, void **extra) int _PyCode_SetExtra(PyObject *code, Py_ssize_t index, void *extra) { - PyInterpreterState *interp = PyThreadState_Get()->interp; + PyCodeExtraState *state = _PyCodeExtraState_Get(); if (!PyCode_Check(code) || index < 0 || - index >= interp->co_extra_user_count) { + index >= state->co_extra_user_count) { PyErr_BadInternalCall(); return -1; } @@ -865,13 +865,13 @@ _PyCode_SetExtra(PyObject *code, Py_ssize_t index, void *extra) } co_extra->ce_extras = PyMem_Malloc( - interp->co_extra_user_count * sizeof(void*)); + state->co_extra_user_count * sizeof(void*)); if (co_extra->ce_extras == NULL) { PyMem_Free(co_extra); return -1; } - co_extra->ce_size = interp->co_extra_user_count; + co_extra->ce_size = state->co_extra_user_count; for (Py_ssize_t i = 0; i < co_extra->ce_size; i++) { co_extra->ce_extras[i] = NULL; @@ -881,24 +881,24 @@ _PyCode_SetExtra(PyObject *code, Py_ssize_t index, void *extra) } else if (co_extra->ce_size <= index) { void** ce_extras = PyMem_Realloc( - co_extra->ce_extras, interp->co_extra_user_count * sizeof(void*)); + co_extra->ce_extras, state->co_extra_user_count * sizeof(void*)); if (ce_extras == NULL) { return -1; } for (Py_ssize_t i = co_extra->ce_size; - i < interp->co_extra_user_count; + i < state->co_extra_user_count; i++) { ce_extras[i] = NULL; } co_extra->ce_extras = ce_extras; - co_extra->ce_size = interp->co_extra_user_count; + co_extra->ce_size = state->co_extra_user_count; } if (co_extra->ce_extras[index] != NULL) { - freefunc free = interp->co_extra_freefuncs[index]; + freefunc free = state->co_extra_freefuncs[index]; if (free != NULL) { free(co_extra->ce_extras[index]); } diff --git a/Python/ceval.c b/Python/ceval.c index 37fb2fc2b25c45..32abb823f86d51 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -5442,14 +5442,14 @@ _Py_GetDXProfile(PyObject *self, PyObject *args) Py_ssize_t _PyEval_RequestCodeExtraIndex(freefunc free) { - PyInterpreterState *interp = PyThreadState_Get()->interp; + PyCodeExtraState *state = _PyCodeExtraState_Get(); Py_ssize_t new_index; - if (interp->co_extra_user_count == MAX_CO_EXTRA_USERS - 1) { + if (state->co_extra_user_count == MAX_CO_EXTRA_USERS - 1) { return -1; } - new_index = interp->co_extra_user_count++; - interp->co_extra_freefuncs[new_index] = free; + new_index = state->co_extra_user_count++; + state->co_extra_freefuncs[new_index] = free; return new_index; } diff --git a/Python/pystate.c b/Python/pystate.c index 9f57e8051eb1d6..6e7ed018f44c4f 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -55,6 +55,7 @@ static int autoTLSkey = -1; #endif static PyInterpreterState *interp_head = NULL; +static PyCodeExtraState *coextra_head = NULL; /* Assuming the current thread holds the GIL, this is the PyThreadState for the current thread. */ @@ -71,8 +72,14 @@ PyInterpreterState_New(void) { PyInterpreterState *interp = (PyInterpreterState *) PyMem_RawMalloc(sizeof(PyInterpreterState)); - + if (interp != NULL) { + PyCodeExtraState* coExtra = PyMem_RawMalloc(sizeof(PyCodeExtraState)); + if (coExtra == NULL) { + PyMem_RawFree(interp); + return NULL; + } + HEAD_INIT(); #ifdef WITH_THREAD if (head_mutex == NULL) @@ -92,7 +99,8 @@ PyInterpreterState_New(void) interp->importlib = NULL; interp->import_func = NULL; interp->eval_frame = _PyEval_EvalFrameDefault; - interp->co_extra_user_count = 0; + coExtra->co_extra_user_count = 0; + coExtra->interp = interp; #ifdef HAVE_DLOPEN #if HAVE_DECL_RTLD_NOW interp->dlopenflags = RTLD_NOW; @@ -104,6 +112,8 @@ PyInterpreterState_New(void) HEAD_LOCK(); interp->next = interp_head; interp_head = interp; + coExtra->next = coextra_head; + coextra_head = coExtra; HEAD_UNLOCK(); } @@ -148,6 +158,7 @@ void PyInterpreterState_Delete(PyInterpreterState *interp) { PyInterpreterState **p; + PyCodeExtraState **pExtra; zapthreads(interp); HEAD_LOCK(); for (p = &interp_head; ; p = &(*p)->next) { @@ -160,6 +171,18 @@ PyInterpreterState_Delete(PyInterpreterState *interp) if (interp->tstate_head != NULL) Py_FatalError("PyInterpreterState_Delete: remaining threads"); *p = interp->next; + + for (pExtra = &coextra_head; ; pExtra = &(*pExtra)->next) { + if (*pExtra == NULL) + Py_FatalError( + "PyInterpreterState_Delete: invalid extra"); + PyCodeExtraState* extra = *pExtra; + if (extra->interp == interp) { + *pExtra = extra->next; + PyMem_RawFree(extra); + break; + } + } HEAD_UNLOCK(); PyMem_RawFree(interp); #ifdef WITH_THREAD @@ -548,6 +571,23 @@ PyThreadState_Swap(PyThreadState *newts) return oldts; } +PyCodeExtraState* +_PyCodeExtraState_Get() { + PyInterpreterState* interp = PyThreadState_Get()->interp; + + HEAD_LOCK(); + for (PyCodeExtraState* cur = coextra_head; cur != NULL; cur = cur->next) { + if (cur->interp == interp) { + HEAD_UNLOCK(); + return cur; + } + } + HEAD_UNLOCK(); + + Py_FatalError("_PyCodeExtraState_Get: no code state for interpreter"); + return NULL; +} + /* An extension mechanism to store arbitrary additional per-thread state. PyThreadState_GetDict() returns a dictionary that can be used to hold such state; the caller should pick a unique key and store its state there. If From f2c604f58b563a264b60f37559a485d1da4cc247 Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Mon, 12 Jun 2017 14:54:13 -0700 Subject: [PATCH 06/13] Rename __PyCodeExtraState_Get and add comment about it going away in 3.7 Fix sort order of import's in test_code.py --- Include/pystate.h | 3 ++- Lib/test/test_code.py | 2 +- Objects/codeobject.c | 4 ++-- Python/ceval.c | 2 +- Python/pystate.c | 4 ++-- 5 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Include/pystate.h b/Include/pystate.h index 8f6f5e2725f018..dfb07186cdc564 100644 --- a/Include/pystate.h +++ b/Include/pystate.h @@ -59,7 +59,8 @@ typedef struct _co_extra_state { freefunc co_extra_freefuncs[MAX_CO_EXTRA_USERS]; } PyCodeExtraState; -PyCodeExtraState* _PyCodeExtraState_Get(); +/* This is temporary for backwards compat in 3.6 and will be removed in 3.7 */ +PyCodeExtraState* __PyCodeExtraState_Get(); /* State unique per thread */ diff --git a/Lib/test/test_code.py b/Lib/test/test_code.py index b6965dc27a98df..62dda0f9ab2dbe 100644 --- a/Lib/test/test_code.py +++ b/Lib/test/test_code.py @@ -103,9 +103,9 @@ """ import sys +import threading import unittest import weakref -import threading from test.support import (run_doctest, run_unittest, cpython_only, check_impl_detail) diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 6245747fa45616..ecd43a19f5825f 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -411,7 +411,7 @@ static void code_dealloc(PyCodeObject *co) { if (co->co_extra != NULL) { - PyCodeExtraState *state = _PyCodeExtraState_Get(); + PyCodeExtraState *state = __PyCodeExtraState_Get(); _PyCodeObjectExtra *co_extra = co->co_extra; for (Py_ssize_t i = 0; i < co_extra->ce_size; i++) { @@ -847,7 +847,7 @@ _PyCode_GetExtra(PyObject *code, Py_ssize_t index, void **extra) int _PyCode_SetExtra(PyObject *code, Py_ssize_t index, void *extra) { - PyCodeExtraState *state = _PyCodeExtraState_Get(); + PyCodeExtraState *state = __PyCodeExtraState_Get(); if (!PyCode_Check(code) || index < 0 || index >= state->co_extra_user_count) { diff --git a/Python/ceval.c b/Python/ceval.c index 32abb823f86d51..662c18ecc60067 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -5442,7 +5442,7 @@ _Py_GetDXProfile(PyObject *self, PyObject *args) Py_ssize_t _PyEval_RequestCodeExtraIndex(freefunc free) { - PyCodeExtraState *state = _PyCodeExtraState_Get(); + PyCodeExtraState *state = __PyCodeExtraState_Get(); Py_ssize_t new_index; if (state->co_extra_user_count == MAX_CO_EXTRA_USERS - 1) { diff --git a/Python/pystate.c b/Python/pystate.c index 6e7ed018f44c4f..c957eac991bada 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -572,7 +572,7 @@ PyThreadState_Swap(PyThreadState *newts) } PyCodeExtraState* -_PyCodeExtraState_Get() { +__PyCodeExtraState_Get() { PyInterpreterState* interp = PyThreadState_Get()->interp; HEAD_LOCK(); @@ -584,7 +584,7 @@ _PyCodeExtraState_Get() { } HEAD_UNLOCK(); - Py_FatalError("_PyCodeExtraState_Get: no code state for interpreter"); + Py_FatalError("__PyCodeExtraState_Get: no code state for interpreter"); return NULL; } From 913f3139bd5e1b9a1cfc46f13b4bb8e77f5d7761 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Mon, 12 Jun 2017 15:06:06 -0700 Subject: [PATCH 07/13] Remove an extraneous space --- Lib/test/test_code.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_code.py b/Lib/test/test_code.py index 62dda0f9ab2dbe..f5eb4aca379bd0 100644 --- a/Lib/test/test_code.py +++ b/Lib/test/test_code.py @@ -220,7 +220,7 @@ def callback(code): freefunc = ctypes.CFUNCTYPE(None,ctypes.c_voidp) RequestCodeExtraIndex = py._PyEval_RequestCodeExtraIndex - RequestCodeExtraIndex.argtypes = (freefunc, ) + RequestCodeExtraIndex.argtypes = (freefunc,) RequestCodeExtraIndex.restype = ctypes.c_ssize_t SetExtra = py._PyCode_SetExtra From 1a05a6a661a7ed5f751da9c7c9b91cc5148a33ac Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Mon, 12 Jun 2017 15:07:45 -0700 Subject: [PATCH 08/13] Remove docstrings for comments --- Lib/test/test_code.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_code.py b/Lib/test/test_code.py index f5eb4aca379bd0..9f9df9dba6b3d6 100644 --- a/Lib/test/test_code.py +++ b/Lib/test/test_code.py @@ -263,8 +263,8 @@ def test_bad_index(self): ctypes.c_voidp(100)), 0) def test_free_called(self): - '''Verify that the provided free function gets invoked - when the code object is cleaned up''' + # Verify that the provided free function gets invoked + # when the code object is cleaned up. f = self.get_func() SetExtra(f.__code__, FREE_INDEX, ctypes.c_voidp(100)) @@ -272,7 +272,7 @@ def test_free_called(self): self.assertEqual(LAST_FREED, 100) def test_get_set(self): - '''Basic get/set round tripping''' + # Test basic get/set round tripping. f = self.get_func() extra = ctypes.c_voidp() @@ -288,8 +288,8 @@ def test_get_set(self): del f def test_free_different_thread(self): - '''Freeing a code object on a different thread then - where the co_extra was set should be safe''' + # Freeing a code object on a different thread then + # where the co_extra was set should be safe. f = self.get_func() class ThreadTest(threading.Thread): def __init__(self, f, test): From 6f0bedb7c7798d39b434f2f93fd7255df37259ed Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Mon, 12 Jun 2017 15:08:53 -0700 Subject: [PATCH 09/13] Touch up formatting --- Python/pystate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/pystate.c b/Python/pystate.c index c957eac991bada..1c8c6213000cad 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -72,7 +72,7 @@ PyInterpreterState_New(void) { PyInterpreterState *interp = (PyInterpreterState *) PyMem_RawMalloc(sizeof(PyInterpreterState)); - + if (interp != NULL) { PyCodeExtraState* coExtra = PyMem_RawMalloc(sizeof(PyCodeExtraState)); if (coExtra == NULL) { @@ -161,7 +161,7 @@ PyInterpreterState_Delete(PyInterpreterState *interp) PyCodeExtraState **pExtra; zapthreads(interp); HEAD_LOCK(); - for (p = &interp_head; ; p = &(*p)->next) { + for (p = &interp_head; /* N/A */; p = &(*p)->next) { if (*p == NULL) Py_FatalError( "PyInterpreterState_Delete: invalid interp"); From b41319a85fcf9298c482dcb5dd37020fb0a5aea9 Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Mon, 12 Jun 2017 15:59:20 -0700 Subject: [PATCH 10/13] Fix casing of coextra local --- Python/pystate.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Python/pystate.c b/Python/pystate.c index 1c8c6213000cad..961b156cbe1f1d 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -74,8 +74,8 @@ PyInterpreterState_New(void) PyMem_RawMalloc(sizeof(PyInterpreterState)); if (interp != NULL) { - PyCodeExtraState* coExtra = PyMem_RawMalloc(sizeof(PyCodeExtraState)); - if (coExtra == NULL) { + PyCodeExtraState* coextra = PyMem_RawMalloc(sizeof(PyCodeExtraState)); + if (coextra == NULL) { PyMem_RawFree(interp); return NULL; } @@ -99,8 +99,8 @@ PyInterpreterState_New(void) interp->importlib = NULL; interp->import_func = NULL; interp->eval_frame = _PyEval_EvalFrameDefault; - coExtra->co_extra_user_count = 0; - coExtra->interp = interp; + coextra->co_extra_user_count = 0; + coextra->interp = interp; #ifdef HAVE_DLOPEN #if HAVE_DECL_RTLD_NOW interp->dlopenflags = RTLD_NOW; @@ -112,8 +112,8 @@ PyInterpreterState_New(void) HEAD_LOCK(); interp->next = interp_head; interp_head = interp; - coExtra->next = coextra_head; - coextra_head = coExtra; + coextra->next = coextra_head; + coextra_head = coextra; HEAD_UNLOCK(); } From 2e23477d0541bdf2ade914ecf533ef84c6685d4c Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Mon, 12 Jun 2017 16:35:51 -0700 Subject: [PATCH 11/13] Fix casing of another variable --- Python/pystate.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Python/pystate.c b/Python/pystate.c index 961b156cbe1f1d..b72f55b4ccf906 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -158,7 +158,7 @@ void PyInterpreterState_Delete(PyInterpreterState *interp) { PyInterpreterState **p; - PyCodeExtraState **pExtra; + PyCodeExtraState **pextra; zapthreads(interp); HEAD_LOCK(); for (p = &interp_head; /* N/A */; p = &(*p)->next) { @@ -172,13 +172,13 @@ PyInterpreterState_Delete(PyInterpreterState *interp) Py_FatalError("PyInterpreterState_Delete: remaining threads"); *p = interp->next; - for (pExtra = &coextra_head; ; pExtra = &(*pExtra)->next) { - if (*pExtra == NULL) + for (pextra = &coextra_head; ; pextra = &(*pextra)->next) { + if (*pextra == NULL) Py_FatalError( "PyInterpreterState_Delete: invalid extra"); - PyCodeExtraState* extra = *pExtra; + PyCodeExtraState* extra = *pextra; if (extra->interp == interp) { - *pExtra = extra->next; + *pextra = extra->next; PyMem_RawFree(extra); break; } From 9473adcaa47c9c05f1e21d39d5998bab36e246b9 Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Mon, 12 Jun 2017 16:52:42 -0700 Subject: [PATCH 12/13] Prefix PyCodeExtraState with __ to match C API for getting it --- Include/pystate.h | 4 ++-- Objects/codeobject.c | 4 ++-- Python/ceval.c | 2 +- Python/pystate.c | 12 ++++++------ 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Include/pystate.h b/Include/pystate.h index dfb07186cdc564..6ca463f7e15a8f 100644 --- a/Include/pystate.h +++ b/Include/pystate.h @@ -57,10 +57,10 @@ typedef struct _co_extra_state { Py_ssize_t co_extra_user_count; freefunc co_extra_freefuncs[MAX_CO_EXTRA_USERS]; -} PyCodeExtraState; +} __PyCodeExtraState; /* This is temporary for backwards compat in 3.6 and will be removed in 3.7 */ -PyCodeExtraState* __PyCodeExtraState_Get(); +__PyCodeExtraState* __PyCodeExtraState_Get(); /* State unique per thread */ diff --git a/Objects/codeobject.c b/Objects/codeobject.c index ecd43a19f5825f..d38f185ba3b8e4 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -411,7 +411,7 @@ static void code_dealloc(PyCodeObject *co) { if (co->co_extra != NULL) { - PyCodeExtraState *state = __PyCodeExtraState_Get(); + __PyCodeExtraState *state = __PyCodeExtraState_Get(); _PyCodeObjectExtra *co_extra = co->co_extra; for (Py_ssize_t i = 0; i < co_extra->ce_size; i++) { @@ -847,7 +847,7 @@ _PyCode_GetExtra(PyObject *code, Py_ssize_t index, void **extra) int _PyCode_SetExtra(PyObject *code, Py_ssize_t index, void *extra) { - PyCodeExtraState *state = __PyCodeExtraState_Get(); + __PyCodeExtraState *state = __PyCodeExtraState_Get(); if (!PyCode_Check(code) || index < 0 || index >= state->co_extra_user_count) { diff --git a/Python/ceval.c b/Python/ceval.c index 662c18ecc60067..3ea1a4f66ba020 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -5442,7 +5442,7 @@ _Py_GetDXProfile(PyObject *self, PyObject *args) Py_ssize_t _PyEval_RequestCodeExtraIndex(freefunc free) { - PyCodeExtraState *state = __PyCodeExtraState_Get(); + __PyCodeExtraState *state = __PyCodeExtraState_Get(); Py_ssize_t new_index; if (state->co_extra_user_count == MAX_CO_EXTRA_USERS - 1) { diff --git a/Python/pystate.c b/Python/pystate.c index b72f55b4ccf906..92d08c41091878 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -55,7 +55,7 @@ static int autoTLSkey = -1; #endif static PyInterpreterState *interp_head = NULL; -static PyCodeExtraState *coextra_head = NULL; +static __PyCodeExtraState *coextra_head = NULL; /* Assuming the current thread holds the GIL, this is the PyThreadState for the current thread. */ @@ -74,7 +74,7 @@ PyInterpreterState_New(void) PyMem_RawMalloc(sizeof(PyInterpreterState)); if (interp != NULL) { - PyCodeExtraState* coextra = PyMem_RawMalloc(sizeof(PyCodeExtraState)); + __PyCodeExtraState* coextra = PyMem_RawMalloc(sizeof(__PyCodeExtraState)); if (coextra == NULL) { PyMem_RawFree(interp); return NULL; @@ -158,7 +158,7 @@ void PyInterpreterState_Delete(PyInterpreterState *interp) { PyInterpreterState **p; - PyCodeExtraState **pextra; + __PyCodeExtraState **pextra; zapthreads(interp); HEAD_LOCK(); for (p = &interp_head; /* N/A */; p = &(*p)->next) { @@ -176,7 +176,7 @@ PyInterpreterState_Delete(PyInterpreterState *interp) if (*pextra == NULL) Py_FatalError( "PyInterpreterState_Delete: invalid extra"); - PyCodeExtraState* extra = *pextra; + __PyCodeExtraState* extra = *pextra; if (extra->interp == interp) { *pextra = extra->next; PyMem_RawFree(extra); @@ -571,12 +571,12 @@ PyThreadState_Swap(PyThreadState *newts) return oldts; } -PyCodeExtraState* +__PyCodeExtraState* __PyCodeExtraState_Get() { PyInterpreterState* interp = PyThreadState_Get()->interp; HEAD_LOCK(); - for (PyCodeExtraState* cur = coextra_head; cur != NULL; cur = cur->next) { + for (__PyCodeExtraState* cur = coextra_head; cur != NULL; cur = cur->next) { if (cur->interp == interp) { HEAD_UNLOCK(); return cur; From 5485fb180cbfaa8010d89fafd7d3d26e5aa24a5f Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Mon, 12 Jun 2017 18:13:11 -0700 Subject: [PATCH 13/13] Update NEWS file for bpo-30604 --- Misc/NEWS | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS b/Misc/NEWS index b819c6db73af2f..7fee2957ac4e0c 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -1,4 +1,4 @@ -+++++++++++ ++++++++++++ Python News +++++++++++ @@ -10,6 +10,8 @@ What's New in Python 3.6.2 release candidate 1? Core and Builtins ----------------- +- bpo-30604: Move co_extra_freefuncs to not be per-thread to avoid crashes + - bpo-29104: Fixed parsing backslashes in f-strings. - bpo-27945: Fixed various segfaults with dict when input collections are