| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent c38ceb0 commit d2be5c7
4 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -100,7 +100,7 @@ extern PyStatus _PyEval_InitGIL(PyThreadState *tstate, int own_gil); | |||
| 100 | 100 | extern void _PyEval_FiniGIL(PyInterpreterState *interp); | |
| 101 | 101 | ||
| 102 | 102 | extern void _PyEval_AcquireLock(PyThreadState *tstate); | |
| 103 | - extern void _PyEval_ReleaseLock(PyThreadState *tstate); | ||
| 103 | + extern void _PyEval_ReleaseLock(PyInterpreterState *, PyThreadState *); | ||
| 104 | 104 | extern PyThreadState * _PyThreadState_SwapNoGIL(PyThreadState *); | |
| 105 | 105 | ||
| 106 | 106 | extern void _PyEval_DeactivateOpCache(void); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -278,6 +278,15 @@ static void recreate_gil(struct _gil_runtime_state *gil) | |||
| 278 | 278 | static void | |
| 279 | 279 | drop_gil(struct _ceval_state *ceval, PyThreadState *tstate) | |
| 280 | 280 | { | |
| 281 | + /* If tstate is NULL, the caller is indicating that we're releasing | ||
| 282 | + the GIL for the last time in this thread. This is particularly | ||
| 283 | + relevant when the current thread state is finalizing or its | ||
| 284 | + interpreter is finalizing (either may be in an inconsistent | ||
| 285 | + state). In that case the current thread will definitely | ||
| 286 | + never try to acquire the GIL again. */ | ||
| 287 | + // XXX It may be more correct to check tstate->_status.finalizing. | ||
| 288 | + // XXX assert(tstate == NULL || !tstate->_status.cleared); | ||
| 289 | + | ||
| 281 | 290 | struct _gil_runtime_state *gil = ceval->gil; | |
| 282 | 291 | if (!_Py_atomic_load_relaxed(&gil->locked)) { | |
| 283 | 292 | Py_FatalError("drop_gil: GIL is not locked"); | |
@@ -298,7 +307,15 @@ drop_gil(struct _ceval_state *ceval, PyThreadState *tstate) | |||
| 298 | 307 | MUTEX_UNLOCK(gil->mutex); | |
| 299 | 308 | ||
| 300 | 309 | #ifdef FORCE_SWITCHING | |
| 301 | - if (_Py_atomic_load_relaxed(&ceval->gil_drop_request) && tstate != NULL) { | ||
| 310 | + /* We check tstate first in case we might be releasing the GIL for | ||
| 311 | + the last time in this thread. In that case there's a possible | ||
| 312 | + race with tstate->interp getting deleted after gil->mutex is | ||
| 313 | + unlocked and before the following code runs, leading to a crash. | ||
| 314 | + We can use (tstate == NULL) to indicate the thread is done with | ||
| 315 | + the GIL, and that's the only time we might delete the | ||
| 316 | + interpreter, so checking tstate first prevents the crash. | ||
| 317 | + See https://github.com/python/cpython/issues/104341. */ | ||
| 318 | + if (tstate != NULL && _Py_atomic_load_relaxed(&ceval->gil_drop_request)) { | ||
| 302 | 319 | MUTEX_LOCK(gil->switch_mutex); | |
| 303 | 320 | /* Not switched yet => wait */ | |
| 304 | 321 | if (((PyThreadState*)_Py_atomic_load_relaxed(&gil->last_holder)) == tstate) | |
@@ -350,6 +367,9 @@ take_gil(PyThreadState *tstate) | |||
| 350 | 367 | int err = errno; | |
| 351 | 368 | ||
| 352 | 369 | assert(tstate != NULL); | |
| 370 | + /* We shouldn't be using a thread state that isn't viable any more. */ | ||
| 371 | + // XXX It may be more correct to check tstate->_status.finalizing. | ||
| 372 | + // XXX assert(!tstate->_status.cleared); | ||
| 353 | 373 | ||
| 354 | 374 | if (tstate_must_exit(tstate)) { | |
| 355 | 375 | /* bpo-39877: If Py_Finalize() has been called and tstate is not the | |
@@ -625,10 +645,12 @@ _PyEval_AcquireLock(PyThreadState *tstate) | |||
| 625 | 645 | } | |
| 626 | 646 | ||
| 627 | 647 | void | |
| 628 | - _PyEval_ReleaseLock(PyThreadState *tstate) | ||
| 648 | + _PyEval_ReleaseLock(PyInterpreterState *interp, PyThreadState *tstate) | ||
| 629 | 649 | { | |
| 630 | - _Py_EnsureTstateNotNULL(tstate); | ||
| 631 | - struct _ceval_state *ceval = &tstate->interp->ceval; | ||
| 650 | + /* If tstate is NULL then we do not expect the current thread | ||
| 651 | + to acquire the GIL ever again. */ | ||
| 652 | + assert(tstate == NULL || tstate->interp == interp); | ||
| 653 | + struct _ceval_state *ceval = &interp->ceval; | ||
| 632 | 654 | drop_gil(ceval, tstate); | |
| 633 | 655 | } | |
| 634 | 656 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -2035,7 +2035,7 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config) | |||
| 2035 | 2035 | const PyConfig *src_config; | |
| 2036 | 2036 | if (save_tstate != NULL) { | |
| 2037 | 2037 | // XXX Might new_interpreter() have been called without the GIL held? | |
| 2038 | - _PyEval_ReleaseLock(save_tstate); | ||
| 2038 | + _PyEval_ReleaseLock(save_tstate->interp, save_tstate); | ||
| 2039 | 2039 | src_config = _PyInterpreterState_GetConfig(save_tstate->interp); | |
| 2040 | 2040 | } | |
| 2041 | 2041 | else | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -822,6 +822,12 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate) | |||
| 822 | 822 | p = p->next; | |
| 823 | 823 | HEAD_UNLOCK(runtime); | |
| 824 | 824 | } | |
| 825 | + if (tstate->interp == interp) { | ||
| 826 | + /* We fix tstate->_status below when we for sure aren't using it | ||
| 827 | + (e.g. no longer need the GIL). */ | ||
| 828 | + // XXX Eliminate the need to do this. | ||
| 829 | + tstate->_status.cleared = 0; | ||
| 830 | + } | ||
| 825 | 831 | ||
| 826 | 832 | /* It is possible that any of the objects below have a finalizer | |
| 827 | 833 | that runs Python code or otherwise relies on a thread state | |
@@ -886,6 +892,12 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate) | |||
| 886 | 892 | Py_CLEAR(interp->builtins); | |
| 887 | 893 | Py_CLEAR(interp->interpreter_trampoline); | |
| 888 | 894 | ||
| 895 | + if (tstate->interp == interp) { | ||
| 896 | + /* We are now safe to fix tstate->_status.cleared. */ | ||
| 897 | + // XXX Do this (much) earlier? | ||
| 898 | + tstate->_status.cleared = 1; | ||
| 899 | + } | ||
| 900 | + | ||
| 889 | 901 | for (int i=0; i < DICT_MAX_WATCHERS; i++) { | |
| 890 | 902 | interp->dict_state.watchers[i] = NULL; | |
| 891 | 903 | } | |
@@ -930,6 +942,7 @@ _PyInterpreterState_Clear(PyThreadState *tstate) | |||
| 930 | 942 | } | |
| 931 | 943 | ||
| 932 | 944 | ||
| 945 | + static inline void tstate_deactivate(PyThreadState *tstate); | ||
| 933 | 946 | static void zapthreads(PyInterpreterState *interp); | |
| 934 | 947 | ||
| 935 | 948 | void | |
@@ -943,7 +956,9 @@ PyInterpreterState_Delete(PyInterpreterState *interp) | |||
| 943 | 956 | PyThreadState *tcur = current_fast_get(runtime); | |
| 944 | 957 | if (tcur != NULL && interp == tcur->interp) { | |
| 945 | 958 | /* Unset current thread. After this, many C API calls become crashy. */ | |
| 946 | - _PyThreadState_Swap(runtime, NULL); | ||
| 959 | + current_fast_clear(runtime); | ||
| 960 | + tstate_deactivate(tcur); | ||
| 961 | + _PyEval_ReleaseLock(interp, NULL); | ||
| 947 | 962 | } | |
| 948 | 963 | ||
| 949 | 964 | zapthreads(interp); | |
@@ -1567,7 +1582,7 @@ _PyThreadState_DeleteCurrent(PyThreadState *tstate) | |||
| 1567 | 1582 | _Py_EnsureTstateNotNULL(tstate); | |
| 1568 | 1583 | tstate_delete_common(tstate); | |
| 1569 | 1584 | current_fast_clear(tstate->interp->runtime); | |
| 1570 | - _PyEval_ReleaseLock(tstate); | ||
| 1585 | + _PyEval_ReleaseLock(tstate->interp, NULL); | ||
| 1571 | 1586 | free_threadstate(tstate); | |
| 1572 | 1587 | } | |
| 1573 | 1588 | ||
@@ -1907,7 +1922,7 @@ _PyThreadState_Swap(_PyRuntimeState *runtime, PyThreadState *newts) | |||
| 1907 | 1922 | { | |
| 1908 | 1923 | PyThreadState *oldts = current_fast_get(runtime); | |
| 1909 | 1924 | if (oldts != NULL) { | |
| 1910 | - _PyEval_ReleaseLock(oldts); | ||
| 1925 | + _PyEval_ReleaseLock(oldts->interp, oldts); | ||
| 1911 | 1926 | } | |
| 1912 | 1927 | _swap_thread_states(runtime, oldts, newts); | |
| 1913 | 1928 | if (newts != NULL) { | |
| Back | FazBrowse Home | New Git URL |
0 commit comments