From 07a09d43cbb98d5939400274beedda0e449c63aa Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 6 Oct 2022 17:19:25 -0600 Subject: [PATCH 01/49] Pass PyInterpreterState to pymalloc_*(). --- Objects/obmalloc.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 276c5a276c06e6..fee142b2f50f5a 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1275,7 +1275,7 @@ allocate_from_new_pool(uint size) or when the max memory limit has been reached. */ static inline void* -pymalloc_alloc(void *Py_UNUSED(ctx), size_t nbytes) +pymalloc_alloc(PyInterpreterState *interp, void *Py_UNUSED(ctx), size_t nbytes) { #ifdef WITH_VALGRIND if (UNLIKELY(running_on_valgrind == -1)) { @@ -1325,7 +1325,8 @@ pymalloc_alloc(void *Py_UNUSED(ctx), size_t nbytes) void * _PyObject_Malloc(void *ctx, size_t nbytes) { - void* ptr = pymalloc_alloc(ctx, nbytes); + PyInterpreterState *interp = _PyInterpreterState_GET(); + void* ptr = pymalloc_alloc(interp, ctx, nbytes); if (LIKELY(ptr != NULL)) { return ptr; } @@ -1344,7 +1345,8 @@ _PyObject_Calloc(void *ctx, size_t nelem, size_t elsize) assert(elsize == 0 || nelem <= (size_t)PY_SSIZE_T_MAX / elsize); size_t nbytes = nelem * elsize; - void* ptr = pymalloc_alloc(ctx, nbytes); + PyInterpreterState *interp = _PyInterpreterState_GET(); + void* ptr = pymalloc_alloc(interp, ctx, nbytes); if (LIKELY(ptr != NULL)) { memset(ptr, 0, nbytes); return ptr; @@ -1545,7 +1547,7 @@ insert_to_freepool(poolp pool) Return 1 if it was freed. Return 0 if the block was not allocated by pymalloc_alloc(). */ static inline int -pymalloc_free(void *Py_UNUSED(ctx), void *p) +pymalloc_free(PyInterpreterState *interp, void *Py_UNUSED(ctx), void *p) { assert(p != NULL); @@ -1610,7 +1612,8 @@ _PyObject_Free(void *ctx, void *p) return; } - if (UNLIKELY(!pymalloc_free(ctx, p))) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (UNLIKELY(!pymalloc_free(interp, ctx, p))) { /* pymalloc didn't allocate this address */ PyMem_RawFree(p); raw_allocated_blocks--; @@ -1628,7 +1631,8 @@ _PyObject_Free(void *ctx, void *p) Return 0 if pymalloc didn't allocated p. */ static int -pymalloc_realloc(void *ctx, void **newptr_p, void *p, size_t nbytes) +pymalloc_realloc(PyInterpreterState *interp, void *ctx, + void **newptr_p, void *p, size_t nbytes) { void *bp; poolp pool; @@ -1697,7 +1701,8 @@ _PyObject_Realloc(void *ctx, void *ptr, size_t nbytes) return _PyObject_Malloc(ctx, nbytes); } - if (pymalloc_realloc(ctx, &ptr2, ptr, nbytes)) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (pymalloc_realloc(interp, ctx, &ptr2, ptr, nbytes)) { return ptr2; } From ca75048cf44fa81004558a14e7d81e3aeb27e1f6 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 7 Oct 2022 13:22:51 -0600 Subject: [PATCH 02/49] Move the object arenas to the interpreter state. --- Include/internal/pycore_interp.h | 5 +- Include/internal/pycore_runtime.h | 2 - Include/internal/pycore_runtime_init.h | 6 +-- Objects/obmalloc.c | 73 ++++++++++++++------------ 4 files changed, 47 insertions(+), 39 deletions(-) diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 0e3d46852f2e6d..7839d6d25aacef 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -21,8 +21,9 @@ extern "C" { #include "pycore_function.h" // FUNC_MAX_WATCHERS #include "pycore_genobject.h" // struct _Py_async_gen_state #include "pycore_gc.h" // struct _gc_runtime_state -#include "pycore_list.h" // struct _Py_list_state #include "pycore_global_objects.h" // struct _Py_interp_static_objects +#include "pycore_list.h" // struct _Py_list_state +#include "pycore_obmalloc.h" // struct obmalloc_state #include "pycore_tuple.h" // struct _Py_tuple_state #include "pycore_typeobject.h" // struct type_cache #include "pycore_unicodeobject.h" // struct _Py_unicode_state @@ -89,6 +90,8 @@ struct _is { int _initialized; int finalizing; + struct _obmalloc_state obmalloc; + struct _ceval_state ceval; struct _gc_runtime_state gc; diff --git a/Include/internal/pycore_runtime.h b/Include/internal/pycore_runtime.h index 9ef270791576e3..fae96750e1f684 100644 --- a/Include/internal/pycore_runtime.h +++ b/Include/internal/pycore_runtime.h @@ -22,7 +22,6 @@ extern "C" { #include "pycore_pymem.h" // struct _pymem_allocators #include "pycore_pyhash.h" // struct pyhash_runtime_state #include "pycore_pythread.h" // struct _pythread_runtime_state -#include "pycore_obmalloc.h" // struct obmalloc_state #include "pycore_signal.h" // struct _signals_runtime_state #include "pycore_time.h" // struct _time_runtime_state #include "pycore_tracemalloc.h" // struct _tracemalloc_runtime_state @@ -88,7 +87,6 @@ typedef struct pyruntimestate { _Py_atomic_address _finalizing; struct _pymem_allocators allocators; - struct _obmalloc_state obmalloc; struct pyhash_runtime_state pyhash_state; struct _time_runtime_state time; struct _pythread_runtime_state threads; diff --git a/Include/internal/pycore_runtime_init.h b/Include/internal/pycore_runtime_init.h index c6a27d076eae2d..6a03fadf7558c9 100644 --- a/Include/internal/pycore_runtime_init.h +++ b/Include/internal/pycore_runtime_init.h @@ -25,7 +25,6 @@ extern "C" { _pymem_allocators_debug_INIT, \ _pymem_allocators_obj_arena_INIT, \ }, \ - .obmalloc = _obmalloc_state_INIT(runtime.obmalloc), \ .pyhash_state = pyhash_state_INIT, \ .signals = _signals_RUNTIME_INIT, \ .interpreters = { \ @@ -94,7 +93,7 @@ extern "C" { }, \ }, \ }, \ - ._main_interpreter = _PyInterpreterState_INIT, \ + ._main_interpreter = _PyInterpreterState_INIT(runtime._main_interpreter), \ } #ifdef HAVE_DLOPEN @@ -110,10 +109,11 @@ extern "C" { # define DLOPENFLAGS_INIT #endif -#define _PyInterpreterState_INIT \ +#define _PyInterpreterState_INIT(interp) \ { \ .id_refcount = -1, \ DLOPENFLAGS_INIT \ + .obmalloc = _obmalloc_state_INIT(interp.obmalloc), \ .ceval = { \ .recursion_limit = Py_DEFAULT_RECURSION_LIMIT, \ }, \ diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index fee142b2f50f5a..97a412b4d29040 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -727,19 +727,22 @@ static int running_on_valgrind = -1; #endif -#define allarenas (_PyRuntime.obmalloc.mgmt.arenas) -#define maxarenas (_PyRuntime.obmalloc.mgmt.maxarenas) -#define unused_arena_objects (_PyRuntime.obmalloc.mgmt.unused_arena_objects) -#define usable_arenas (_PyRuntime.obmalloc.mgmt.usable_arenas) -#define nfp2lasta (_PyRuntime.obmalloc.mgmt.nfp2lasta) -#define narenas_currently_allocated (_PyRuntime.obmalloc.mgmt.narenas_currently_allocated) -#define ntimes_arena_allocated (_PyRuntime.obmalloc.mgmt.ntimes_arena_allocated) -#define narenas_highwater (_PyRuntime.obmalloc.mgmt.narenas_highwater) -#define raw_allocated_blocks (_PyRuntime.obmalloc.mgmt.raw_allocated_blocks) +// These macros all rely on a local "interp" variable. +#define usedpools (interp->obmalloc.pools.used) +#define allarenas (interp->obmalloc.mgmt.arenas) +#define maxarenas (interp->obmalloc.mgmt.maxarenas) +#define unused_arena_objects (interp->obmalloc.mgmt.unused_arena_objects) +#define usable_arenas (interp->obmalloc.mgmt.usable_arenas) +#define nfp2lasta (interp->obmalloc.mgmt.nfp2lasta) +#define narenas_currently_allocated (interp->obmalloc.mgmt.narenas_currently_allocated) +#define ntimes_arena_allocated (interp->obmalloc.mgmt.ntimes_arena_allocated) +#define narenas_highwater (interp->obmalloc.mgmt.narenas_highwater) +#define raw_allocated_blocks (interp->obmalloc.mgmt.raw_allocated_blocks) Py_ssize_t _Py_GetAllocatedBlocks(void) { + PyInterpreterState *interp = _PyInterpreterState_GET(); Py_ssize_t n = raw_allocated_blocks; /* add up allocated blocks for used pools */ for (uint i = 0; i < maxarenas; ++i) { @@ -764,16 +767,16 @@ _Py_GetAllocatedBlocks(void) /*==========================================================================*/ /* radix tree for tracking arena usage. */ -#define arena_map_root (_PyRuntime.obmalloc.usage.arena_map_root) +#define arena_map_root (interp->obmalloc.usage.arena_map_root) #ifdef USE_INTERIOR_NODES -#define arena_map_mid_count (_PyRuntime.obmalloc.usage.arena_map_mid_count) -#define arena_map_bot_count (_PyRuntime.obmalloc.usage.arena_map_bot_count) +#define arena_map_mid_count (interp->obmalloc.usage.arena_map_mid_count) +#define arena_map_bot_count (interp->obmalloc.usage.arena_map_bot_count) #endif /* Return a pointer to a bottom tree node, return NULL if it doesn't exist or * it cannot be created */ static Py_ALWAYS_INLINE arena_map_bot_t * -arena_map_get(pymem_block *p, int create) +arena_map_get(PyInterpreterState *interp, pymem_block *p, int create) { #ifdef USE_INTERIOR_NODES /* sanity check that IGNORE_BITS is correct */ @@ -834,11 +837,13 @@ arena_map_get(pymem_block *p, int create) /* mark or unmark addresses covered by arena */ static int -arena_map_mark_used(uintptr_t arena_base, int is_used) +arena_map_mark_used(PyInterpreterState *interp, + uintptr_t arena_base, int is_used) { /* sanity check that IGNORE_BITS is correct */ assert(HIGH_BITS(arena_base) == HIGH_BITS(&arena_map_root)); - arena_map_bot_t *n_hi = arena_map_get((pymem_block *)arena_base, is_used); + arena_map_bot_t *n_hi = arena_map_get( + interp, (pymem_block *)arena_base, is_used); if (n_hi == NULL) { assert(is_used); /* otherwise node should already exist */ return 0; /* failed to allocate space for node */ @@ -863,7 +868,8 @@ arena_map_mark_used(uintptr_t arena_base, int is_used) * must overflow to 0. However, that would mean arena_base was * "ideal" and we should not be in this case. */ assert(arena_base < arena_base_next); - arena_map_bot_t *n_lo = arena_map_get((pymem_block *)arena_base_next, is_used); + arena_map_bot_t *n_lo = arena_map_get( + interp, (pymem_block *)arena_base_next, is_used); if (n_lo == NULL) { assert(is_used); /* otherwise should already exist */ n_hi->arenas[i3].tail_hi = 0; @@ -878,9 +884,9 @@ arena_map_mark_used(uintptr_t arena_base, int is_used) /* Return true if 'p' is a pointer inside an obmalloc arena. * _PyObject_Free() calls this so it needs to be very fast. */ static int -arena_map_is_used(pymem_block *p) +arena_map_is_used(PyInterpreterState *interp, pymem_block *p) { - arena_map_bot_t *n = arena_map_get(p, 0); + arena_map_bot_t *n = arena_map_get(interp, p, 0); if (n == NULL) { return 0; } @@ -903,7 +909,7 @@ arena_map_is_used(pymem_block *p) * `usable_arenas` to the return value. */ static struct arena_object* -new_arena(void) +new_arena(PyInterpreterState *interp) { struct arena_object* arenaobj; uint excess; /* number of bytes above pool alignment */ @@ -969,7 +975,7 @@ new_arena(void) address = _PyObject_Arena.alloc(_PyObject_Arena.ctx, ARENA_SIZE); #if WITH_PYMALLOC_RADIX_TREE if (address != NULL) { - if (!arena_map_mark_used((uintptr_t)address, 1)) { + if (!arena_map_mark_used(interp, (uintptr_t)address, 1)) { /* marking arena in radix tree failed, abort */ _PyObject_Arena.free(_PyObject_Arena.ctx, address, ARENA_SIZE); address = NULL; @@ -1012,9 +1018,9 @@ new_arena(void) pymalloc. When the radix tree is used, 'poolp' is unused. */ static bool -address_in_range(void *p, poolp Py_UNUSED(pool)) +address_in_range(PyInterpreterState *interp, void *p, poolp Py_UNUSED(pool)) { - return arena_map_is_used(p); + return arena_map_is_used(interp, p); } #else /* @@ -1095,7 +1101,7 @@ extremely desirable that it be this fast. static bool _Py_NO_SANITIZE_ADDRESS _Py_NO_SANITIZE_THREAD _Py_NO_SANITIZE_MEMORY -address_in_range(void *p, poolp pool) +address_in_range(PyInterpreterState *interp, void *p, poolp pool) { // Since address_in_range may be reading from memory which was not allocated // by Python, it is important that pool->arenaindex is read only once, as @@ -1139,7 +1145,7 @@ pymalloc_pool_extend(poolp pool, uint size) * This function takes new pool and allocate a block from it. */ static void* -allocate_from_new_pool(uint size) +allocate_from_new_pool(PyInterpreterState *interp, uint size) { /* There isn't a pool of the right size class immediately * available: use a free pool. @@ -1151,7 +1157,7 @@ allocate_from_new_pool(uint size) return NULL; } #endif - usable_arenas = new_arena(); + usable_arenas = new_arena(interp); if (usable_arenas == NULL) { return NULL; } @@ -1315,7 +1321,7 @@ pymalloc_alloc(PyInterpreterState *interp, void *Py_UNUSED(ctx), size_t nbytes) /* There isn't a pool of the right size class immediately * available: use a free pool. */ - bp = allocate_from_new_pool(size); + bp = allocate_from_new_pool(interp, size); } return (void *)bp; @@ -1361,7 +1367,7 @@ _PyObject_Calloc(void *ctx, size_t nelem, size_t elsize) static void -insert_to_usedpool(poolp pool) +insert_to_usedpool(PyInterpreterState *interp, poolp pool) { assert(pool->ref.count > 0); /* else the pool is empty */ @@ -1377,7 +1383,7 @@ insert_to_usedpool(poolp pool) } static void -insert_to_freepool(poolp pool) +insert_to_freepool(PyInterpreterState *interp, poolp pool) { poolp next = pool->nextpool; poolp prev = pool->prevpool; @@ -1460,7 +1466,7 @@ insert_to_freepool(poolp pool) #if WITH_PYMALLOC_RADIX_TREE /* mark arena region as not under control of obmalloc */ - arena_map_mark_used(ao->address, 0); + arena_map_mark_used(interp, ao->address, 0); #endif /* Free the entire arena. */ @@ -1558,7 +1564,7 @@ pymalloc_free(PyInterpreterState *interp, void *Py_UNUSED(ctx), void *p) #endif poolp pool = POOL_ADDR(p); - if (UNLIKELY(!address_in_range(p, pool))) { + if (UNLIKELY(!address_in_range(interp, p, pool))) { return 0; } /* We allocated this address. */ @@ -1582,7 +1588,7 @@ pymalloc_free(PyInterpreterState *interp, void *Py_UNUSED(ctx), void *p) * targets optimal filling when several pools contain * blocks of the same size class. */ - insert_to_usedpool(pool); + insert_to_usedpool(interp, pool); return 1; } @@ -1599,7 +1605,7 @@ pymalloc_free(PyInterpreterState *interp, void *Py_UNUSED(ctx), void *p) * previously freed pools will be allocated later * (being not referenced, they are perhaps paged out). */ - insert_to_freepool(pool); + insert_to_freepool(interp, pool); return 1; } @@ -1648,7 +1654,7 @@ pymalloc_realloc(PyInterpreterState *interp, void *ctx, #endif pool = POOL_ADDR(p); - if (!address_in_range(p, pool)) { + if (!address_in_range(interp, p, pool)) { /* pymalloc is not managing this block. If nbytes <= SMALL_REQUEST_THRESHOLD, it's tempting to try to take @@ -2295,6 +2301,7 @@ _PyObject_DebugMallocStats(FILE *out) if (!_PyMem_PymallocEnabled()) { return 0; } + PyInterpreterState *interp = _PyInterpreterState_GET(); uint i; const uint numclasses = SMALL_REQUEST_THRESHOLD >> ALIGNMENT_SHIFT; From 4ee199b166365a0222a12c3a12c47127032ce40e Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 7 Feb 2023 12:17:27 -0700 Subject: [PATCH 03/49] Drop an errant #define. --- Objects/obmalloc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 97a412b4d29040..1a19764eb0ca3e 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1118,8 +1118,6 @@ address_in_range(PyInterpreterState *interp, void *p, poolp pool) /*==========================================================================*/ -#define usedpools (_PyRuntime.obmalloc.pools.used) - // Called when freelist is exhausted. Extend the freelist if there is // space for a block. Otherwise, remove this pool from usedpools. static void From 2768fa44a765900493a20f4b5825b50571dc4d0d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 7 Feb 2023 12:18:03 -0700 Subject: [PATCH 04/49] Leave dump_debug_stats in the global state. --- Include/internal/pycore_obmalloc.h | 5 ++++- Include/internal/pycore_obmalloc_init.h | 6 +++++- Include/internal/pycore_runtime.h | 1 + Include/internal/pycore_runtime_init.h | 1 + 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_obmalloc.h b/Include/internal/pycore_obmalloc.h index a5c7f4528f9126..e9a95f4ebc742c 100644 --- a/Include/internal/pycore_obmalloc.h +++ b/Include/internal/pycore_obmalloc.h @@ -657,8 +657,11 @@ struct _obmalloc_usage { #endif /* WITH_PYMALLOC_RADIX_TREE */ -struct _obmalloc_state { +struct _obmalloc_global_state { int dump_debug_stats; +}; + +struct _obmalloc_state { struct _obmalloc_pools pools; struct _obmalloc_mgmt mgmt; struct _obmalloc_usage usage; diff --git a/Include/internal/pycore_obmalloc_init.h b/Include/internal/pycore_obmalloc_init.h index c9f197e72de9f5..8ee72ff2d4126f 100644 --- a/Include/internal/pycore_obmalloc_init.h +++ b/Include/internal/pycore_obmalloc_init.h @@ -54,9 +54,13 @@ extern "C" { # error "NB_SMALL_SIZE_CLASSES should be less than 64" #endif -#define _obmalloc_state_INIT(obmalloc) \ +#define _obmalloc_global_state_INIT \ { \ .dump_debug_stats = -1, \ + } + +#define _obmalloc_state_INIT(obmalloc) \ + { \ .pools = { \ .used = _obmalloc_pools_INIT(obmalloc.pools), \ }, \ diff --git a/Include/internal/pycore_runtime.h b/Include/internal/pycore_runtime.h index fae96750e1f684..1b1f7170798f2e 100644 --- a/Include/internal/pycore_runtime.h +++ b/Include/internal/pycore_runtime.h @@ -87,6 +87,7 @@ typedef struct pyruntimestate { _Py_atomic_address _finalizing; struct _pymem_allocators allocators; + struct _obmalloc_global_state obmalloc; struct pyhash_runtime_state pyhash_state; struct _time_runtime_state time; struct _pythread_runtime_state threads; diff --git a/Include/internal/pycore_runtime_init.h b/Include/internal/pycore_runtime_init.h index 6a03fadf7558c9..df68a2d2276655 100644 --- a/Include/internal/pycore_runtime_init.h +++ b/Include/internal/pycore_runtime_init.h @@ -25,6 +25,7 @@ extern "C" { _pymem_allocators_debug_INIT, \ _pymem_allocators_obj_arena_INIT, \ }, \ + .obmalloc = _obmalloc_global_state_INIT, \ .pyhash_state = pyhash_state_INIT, \ .signals = _signals_RUNTIME_INIT, \ .interpreters = { \ From bf9425fae8bab3e024de7241840c83686fb00ab1 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 9 Feb 2023 08:47:32 -0700 Subject: [PATCH 05/49] Dynamically initialize obmalloc for subinterpreters. --- Python/pystate.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Python/pystate.c b/Python/pystate.c index ed8c2e212a5539..e8cbc09a4b0238 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -638,6 +638,14 @@ init_interpreter(PyInterpreterState *interp, assert(next != NULL || (interp == runtime->interpreters.main)); interp->next = next; + /* Initialize obmalloc, but only for subinterpreters, + since the main interpreter is initialized statically. */ + if (interp != &runtime->_main_interpreter) { + poolp temp[OBMALLOC_USED_POOLS_SIZE] = \ + _obmalloc_pools_INIT(interp->obmalloc.pools); + memcpy(&interp->obmalloc.pools.used, temp, sizeof(temp)); + } + _PyEval_InitState(&interp->ceval, pending_lock); _PyGC_InitState(&interp->gc); PyConfig_InitPythonConfig(&interp->config); From 6c3111c8029e853807db322d3654ce4ecedbcf35 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 8 Mar 2023 16:55:31 -0700 Subject: [PATCH 06/49] Pass around struct _obmalloc_state* instead of PyInterpeterState*. --- Objects/obmalloc.c | 106 ++++++++++++++++++++++++--------------------- 1 file changed, 57 insertions(+), 49 deletions(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 1a19764eb0ca3e..29c732ba5d9987 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -726,23 +726,32 @@ PyObject_Free(void *ptr) static int running_on_valgrind = -1; #endif +typedef struct _obmalloc_state OMState; -// These macros all rely on a local "interp" variable. -#define usedpools (interp->obmalloc.pools.used) -#define allarenas (interp->obmalloc.mgmt.arenas) -#define maxarenas (interp->obmalloc.mgmt.maxarenas) -#define unused_arena_objects (interp->obmalloc.mgmt.unused_arena_objects) -#define usable_arenas (interp->obmalloc.mgmt.usable_arenas) -#define nfp2lasta (interp->obmalloc.mgmt.nfp2lasta) -#define narenas_currently_allocated (interp->obmalloc.mgmt.narenas_currently_allocated) -#define ntimes_arena_allocated (interp->obmalloc.mgmt.ntimes_arena_allocated) -#define narenas_highwater (interp->obmalloc.mgmt.narenas_highwater) -#define raw_allocated_blocks (interp->obmalloc.mgmt.raw_allocated_blocks) +static inline OMState * +get_state(void) +{ + PyInterpreterState *interp = _PyInterpreterState_GET(); + return &interp->obmalloc; +} + +// These macros all rely on a local "state" variable. +#define usedpools (state->pools.used) +#define allarenas (state->mgmt.arenas) +#define maxarenas (state->mgmt.maxarenas) +#define unused_arena_objects (state->mgmt.unused_arena_objects) +#define usable_arenas (state->mgmt.usable_arenas) +#define nfp2lasta (state->mgmt.nfp2lasta) +#define narenas_currently_allocated (state->mgmt.narenas_currently_allocated) +#define ntimes_arena_allocated (state->mgmt.ntimes_arena_allocated) +#define narenas_highwater (state->mgmt.narenas_highwater) +#define raw_allocated_blocks (state->mgmt.raw_allocated_blocks) Py_ssize_t _Py_GetAllocatedBlocks(void) { - PyInterpreterState *interp = _PyInterpreterState_GET(); + OMState *state = get_state(); + Py_ssize_t n = raw_allocated_blocks; /* add up allocated blocks for used pools */ for (uint i = 0; i < maxarenas; ++i) { @@ -767,16 +776,16 @@ _Py_GetAllocatedBlocks(void) /*==========================================================================*/ /* radix tree for tracking arena usage. */ -#define arena_map_root (interp->obmalloc.usage.arena_map_root) +#define arena_map_root (state->usage.arena_map_root) #ifdef USE_INTERIOR_NODES -#define arena_map_mid_count (interp->obmalloc.usage.arena_map_mid_count) -#define arena_map_bot_count (interp->obmalloc.usage.arena_map_bot_count) +#define arena_map_mid_count (state->usage.arena_map_mid_count) +#define arena_map_bot_count (state->usage.arena_map_bot_count) #endif /* Return a pointer to a bottom tree node, return NULL if it doesn't exist or * it cannot be created */ static Py_ALWAYS_INLINE arena_map_bot_t * -arena_map_get(PyInterpreterState *interp, pymem_block *p, int create) +arena_map_get(OMState *state, pymem_block *p, int create) { #ifdef USE_INTERIOR_NODES /* sanity check that IGNORE_BITS is correct */ @@ -837,13 +846,12 @@ arena_map_get(PyInterpreterState *interp, pymem_block *p, int create) /* mark or unmark addresses covered by arena */ static int -arena_map_mark_used(PyInterpreterState *interp, - uintptr_t arena_base, int is_used) +arena_map_mark_used(OMState *state, uintptr_t arena_base, int is_used) { /* sanity check that IGNORE_BITS is correct */ assert(HIGH_BITS(arena_base) == HIGH_BITS(&arena_map_root)); arena_map_bot_t *n_hi = arena_map_get( - interp, (pymem_block *)arena_base, is_used); + state, (pymem_block *)arena_base, is_used); if (n_hi == NULL) { assert(is_used); /* otherwise node should already exist */ return 0; /* failed to allocate space for node */ @@ -869,7 +877,7 @@ arena_map_mark_used(PyInterpreterState *interp, * "ideal" and we should not be in this case. */ assert(arena_base < arena_base_next); arena_map_bot_t *n_lo = arena_map_get( - interp, (pymem_block *)arena_base_next, is_used); + state, (pymem_block *)arena_base_next, is_used); if (n_lo == NULL) { assert(is_used); /* otherwise should already exist */ n_hi->arenas[i3].tail_hi = 0; @@ -884,9 +892,9 @@ arena_map_mark_used(PyInterpreterState *interp, /* Return true if 'p' is a pointer inside an obmalloc arena. * _PyObject_Free() calls this so it needs to be very fast. */ static int -arena_map_is_used(PyInterpreterState *interp, pymem_block *p) +arena_map_is_used(OMState *state, pymem_block *p) { - arena_map_bot_t *n = arena_map_get(interp, p, 0); + arena_map_bot_t *n = arena_map_get(state, p, 0); if (n == NULL) { return 0; } @@ -909,7 +917,7 @@ arena_map_is_used(PyInterpreterState *interp, pymem_block *p) * `usable_arenas` to the return value. */ static struct arena_object* -new_arena(PyInterpreterState *interp) +new_arena(OMState *state) { struct arena_object* arenaobj; uint excess; /* number of bytes above pool alignment */ @@ -975,7 +983,7 @@ new_arena(PyInterpreterState *interp) address = _PyObject_Arena.alloc(_PyObject_Arena.ctx, ARENA_SIZE); #if WITH_PYMALLOC_RADIX_TREE if (address != NULL) { - if (!arena_map_mark_used(interp, (uintptr_t)address, 1)) { + if (!arena_map_mark_used(state, (uintptr_t)address, 1)) { /* marking arena in radix tree failed, abort */ _PyObject_Arena.free(_PyObject_Arena.ctx, address, ARENA_SIZE); address = NULL; @@ -1018,9 +1026,9 @@ new_arena(PyInterpreterState *interp) pymalloc. When the radix tree is used, 'poolp' is unused. */ static bool -address_in_range(PyInterpreterState *interp, void *p, poolp Py_UNUSED(pool)) +address_in_range(OMState *state, void *p, poolp Py_UNUSED(pool)) { - return arena_map_is_used(interp, p); + return arena_map_is_used(state, p); } #else /* @@ -1101,7 +1109,7 @@ extremely desirable that it be this fast. static bool _Py_NO_SANITIZE_ADDRESS _Py_NO_SANITIZE_THREAD _Py_NO_SANITIZE_MEMORY -address_in_range(PyInterpreterState *interp, void *p, poolp pool) +address_in_range(OMState *state, void *p, poolp pool) { // Since address_in_range may be reading from memory which was not allocated // by Python, it is important that pool->arenaindex is read only once, as @@ -1143,7 +1151,7 @@ pymalloc_pool_extend(poolp pool, uint size) * This function takes new pool and allocate a block from it. */ static void* -allocate_from_new_pool(PyInterpreterState *interp, uint size) +allocate_from_new_pool(OMState *state, uint size) { /* There isn't a pool of the right size class immediately * available: use a free pool. @@ -1155,7 +1163,7 @@ allocate_from_new_pool(PyInterpreterState *interp, uint size) return NULL; } #endif - usable_arenas = new_arena(interp); + usable_arenas = new_arena(state); if (usable_arenas == NULL) { return NULL; } @@ -1279,7 +1287,7 @@ allocate_from_new_pool(PyInterpreterState *interp, uint size) or when the max memory limit has been reached. */ static inline void* -pymalloc_alloc(PyInterpreterState *interp, void *Py_UNUSED(ctx), size_t nbytes) +pymalloc_alloc(OMState *state, void *Py_UNUSED(ctx), size_t nbytes) { #ifdef WITH_VALGRIND if (UNLIKELY(running_on_valgrind == -1)) { @@ -1319,7 +1327,7 @@ pymalloc_alloc(PyInterpreterState *interp, void *Py_UNUSED(ctx), size_t nbytes) /* There isn't a pool of the right size class immediately * available: use a free pool. */ - bp = allocate_from_new_pool(interp, size); + bp = allocate_from_new_pool(state, size); } return (void *)bp; @@ -1329,8 +1337,8 @@ pymalloc_alloc(PyInterpreterState *interp, void *Py_UNUSED(ctx), size_t nbytes) void * _PyObject_Malloc(void *ctx, size_t nbytes) { - PyInterpreterState *interp = _PyInterpreterState_GET(); - void* ptr = pymalloc_alloc(interp, ctx, nbytes); + OMState *state = get_state(); + void* ptr = pymalloc_alloc(state, ctx, nbytes); if (LIKELY(ptr != NULL)) { return ptr; } @@ -1349,8 +1357,8 @@ _PyObject_Calloc(void *ctx, size_t nelem, size_t elsize) assert(elsize == 0 || nelem <= (size_t)PY_SSIZE_T_MAX / elsize); size_t nbytes = nelem * elsize; - PyInterpreterState *interp = _PyInterpreterState_GET(); - void* ptr = pymalloc_alloc(interp, ctx, nbytes); + OMState *state = get_state(); + void* ptr = pymalloc_alloc(state, ctx, nbytes); if (LIKELY(ptr != NULL)) { memset(ptr, 0, nbytes); return ptr; @@ -1365,7 +1373,7 @@ _PyObject_Calloc(void *ctx, size_t nelem, size_t elsize) static void -insert_to_usedpool(PyInterpreterState *interp, poolp pool) +insert_to_usedpool(OMState *state, poolp pool) { assert(pool->ref.count > 0); /* else the pool is empty */ @@ -1381,7 +1389,7 @@ insert_to_usedpool(PyInterpreterState *interp, poolp pool) } static void -insert_to_freepool(PyInterpreterState *interp, poolp pool) +insert_to_freepool(OMState *state, poolp pool) { poolp next = pool->nextpool; poolp prev = pool->prevpool; @@ -1464,7 +1472,7 @@ insert_to_freepool(PyInterpreterState *interp, poolp pool) #if WITH_PYMALLOC_RADIX_TREE /* mark arena region as not under control of obmalloc */ - arena_map_mark_used(interp, ao->address, 0); + arena_map_mark_used(state, ao->address, 0); #endif /* Free the entire arena. */ @@ -1551,7 +1559,7 @@ insert_to_freepool(PyInterpreterState *interp, poolp pool) Return 1 if it was freed. Return 0 if the block was not allocated by pymalloc_alloc(). */ static inline int -pymalloc_free(PyInterpreterState *interp, void *Py_UNUSED(ctx), void *p) +pymalloc_free(OMState *state, void *Py_UNUSED(ctx), void *p) { assert(p != NULL); @@ -1562,7 +1570,7 @@ pymalloc_free(PyInterpreterState *interp, void *Py_UNUSED(ctx), void *p) #endif poolp pool = POOL_ADDR(p); - if (UNLIKELY(!address_in_range(interp, p, pool))) { + if (UNLIKELY(!address_in_range(state, p, pool))) { return 0; } /* We allocated this address. */ @@ -1586,7 +1594,7 @@ pymalloc_free(PyInterpreterState *interp, void *Py_UNUSED(ctx), void *p) * targets optimal filling when several pools contain * blocks of the same size class. */ - insert_to_usedpool(interp, pool); + insert_to_usedpool(state, pool); return 1; } @@ -1603,7 +1611,7 @@ pymalloc_free(PyInterpreterState *interp, void *Py_UNUSED(ctx), void *p) * previously freed pools will be allocated later * (being not referenced, they are perhaps paged out). */ - insert_to_freepool(interp, pool); + insert_to_freepool(state, pool); return 1; } @@ -1616,8 +1624,8 @@ _PyObject_Free(void *ctx, void *p) return; } - PyInterpreterState *interp = _PyInterpreterState_GET(); - if (UNLIKELY(!pymalloc_free(interp, ctx, p))) { + OMState *state = get_state(); + if (UNLIKELY(!pymalloc_free(state, ctx, p))) { /* pymalloc didn't allocate this address */ PyMem_RawFree(p); raw_allocated_blocks--; @@ -1635,7 +1643,7 @@ _PyObject_Free(void *ctx, void *p) Return 0 if pymalloc didn't allocated p. */ static int -pymalloc_realloc(PyInterpreterState *interp, void *ctx, +pymalloc_realloc(OMState *state, void *ctx, void **newptr_p, void *p, size_t nbytes) { void *bp; @@ -1652,7 +1660,7 @@ pymalloc_realloc(PyInterpreterState *interp, void *ctx, #endif pool = POOL_ADDR(p); - if (!address_in_range(interp, p, pool)) { + if (!address_in_range(state, p, pool)) { /* pymalloc is not managing this block. If nbytes <= SMALL_REQUEST_THRESHOLD, it's tempting to try to take @@ -1705,8 +1713,8 @@ _PyObject_Realloc(void *ctx, void *ptr, size_t nbytes) return _PyObject_Malloc(ctx, nbytes); } - PyInterpreterState *interp = _PyInterpreterState_GET(); - if (pymalloc_realloc(interp, ctx, &ptr2, ptr, nbytes)) { + OMState *state = get_state(); + if (pymalloc_realloc(state, ctx, &ptr2, ptr, nbytes)) { return ptr2; } @@ -2299,7 +2307,7 @@ _PyObject_DebugMallocStats(FILE *out) if (!_PyMem_PymallocEnabled()) { return 0; } - PyInterpreterState *interp = _PyInterpreterState_GET(); + OMState *state = get_state(); uint i; const uint numclasses = SMALL_REQUEST_THRESHOLD >> ALIGNMENT_SHIFT; From 4dc087dea276b3f5c0b8b98b79614661c777971d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 8 Mar 2023 17:18:51 -0700 Subject: [PATCH 07/49] Add _PyInterpreterConfig.use_main_obmalloc. --- Include/cpython/initconfig.h | 3 +++ Include/cpython/pystate.h | 4 ++++ Lib/test/test_capi/test_misc.py | 17 +++++++++++------ Lib/test/test_embed.py | 3 ++- Lib/test/test_threading.py | 1 + Modules/_testcapimodule.c | 12 ++++++++++-- Python/pylifecycle.c | 22 +++++++++++++++++++--- 7 files changed, 50 insertions(+), 12 deletions(-) diff --git a/Include/cpython/initconfig.h b/Include/cpython/initconfig.h index a070fa9ff3a038..10c3c3bf502745 100644 --- a/Include/cpython/initconfig.h +++ b/Include/cpython/initconfig.h @@ -244,6 +244,7 @@ PyAPI_FUNC(PyStatus) PyConfig_SetWideStringList(PyConfig *config, /* --- PyInterpreterConfig ------------------------------------ */ typedef struct { + int use_main_obmalloc; int allow_fork; int allow_exec; int allow_threads; @@ -253,6 +254,7 @@ typedef struct { #define _PyInterpreterConfig_INIT \ { \ + .use_main_obmalloc = 0, \ .allow_fork = 0, \ .allow_exec = 0, \ .allow_threads = 1, \ @@ -262,6 +264,7 @@ typedef struct { #define _PyInterpreterConfig_LEGACY_INIT \ { \ + .use_main_obmalloc = 1, \ .allow_fork = 1, \ .allow_exec = 1, \ .allow_threads = 1, \ diff --git a/Include/cpython/pystate.h b/Include/cpython/pystate.h index 3efb241e8237e7..5ed81464293310 100644 --- a/Include/cpython/pystate.h +++ b/Include/cpython/pystate.h @@ -11,6 +11,10 @@ is available in a given context. For example, forking the process might not be allowed in the current interpreter (i.e. os.fork() would fail). */ +/* Set if the interpreter share obmalloc runtime state + with the main interpreter. */ +#define Py_RTFLAGS_USE_MAIN_OBMALLOC (1UL << 5) + /* Set if import should check a module for subinterpreter support. */ #define Py_RTFLAGS_MULTI_INTERP_EXTENSIONS (1UL << 8) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index c34ee578b5c83f..40f7d24f487e48 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -1215,20 +1215,23 @@ def test_configured_settings(self): """ import json + OBMALLOC = 1<<5 EXTENSIONS = 1<<8 THREADS = 1<<10 DAEMON_THREADS = 1<<11 FORK = 1<<15 EXEC = 1<<16 - features = ['fork', 'exec', 'threads', 'daemon_threads', 'extensions'] + features = ['obmalloc', 'fork', 'exec', 'threads', 'daemon_threads', + 'extensions'] kwlist = [f'allow_{n}' for n in features] + kwlist[0] = 'use_main_obmalloc' kwlist[-1] = 'check_multi_interp_extensions' for config, expected in { - (True, True, True, True, True): - FORK | EXEC | THREADS | DAEMON_THREADS | EXTENSIONS, - (False, False, False, False, False): 0, - (False, False, True, False, True): THREADS | EXTENSIONS, + (True, True, True, True, True, True): + OBMALLOC | FORK | EXEC | THREADS | DAEMON_THREADS | EXTENSIONS, + (False, False, False, False, False, False): 0, + (False, False, False, True, False, True): THREADS | EXTENSIONS, }.items(): kwargs = dict(zip(kwlist, config)) expected = { @@ -1261,13 +1264,15 @@ def test_overridden_setting_extensions_subinterp_check(self): """ import json + OBMALLOC = 1<<5 EXTENSIONS = 1<<8 THREADS = 1<<10 DAEMON_THREADS = 1<<11 FORK = 1<<15 EXEC = 1<<16 - BASE_FLAGS = FORK | EXEC | THREADS | DAEMON_THREADS + BASE_FLAGS = OBMALLOC | FORK | EXEC | THREADS | DAEMON_THREADS base_kwargs = { + 'use_main_obmalloc': True, 'allow_fork': True, 'allow_exec': True, 'allow_threads': True, diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index e56d0db8627e91..f702ffb99905a5 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -1656,6 +1656,7 @@ def test_init_use_frozen_modules(self): api=API_PYTHON, env=env) def test_init_main_interpreter_settings(self): + OBMALLOC = 1<<5 EXTENSIONS = 1<<8 THREADS = 1<<10 DAEMON_THREADS = 1<<11 @@ -1664,7 +1665,7 @@ def test_init_main_interpreter_settings(self): expected = { # All optional features should be enabled. 'feature_flags': - FORK | EXEC | THREADS | DAEMON_THREADS, + OBMALLOC | FORK | EXEC | THREADS | DAEMON_THREADS, } out, err = self.run_embedded_interpreter( 'test_init_main_interpreter_settings', diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index a39a267b403d83..fdd74c37e26235 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -1343,6 +1343,7 @@ def func(): import test.support test.support.run_in_subinterp_with_config( {subinterp_code!r}, + use_main_obmalloc=True, allow_fork=True, allow_exec=True, allow_threads={allowed}, diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index ea67017a1ba3b1..329e3e92aa4c02 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -1482,6 +1482,7 @@ static PyObject * run_in_subinterp_with_config(PyObject *self, PyObject *args, PyObject *kwargs) { const char *code; + int use_main_obmalloc = -1; int allow_fork = -1; int allow_exec = -1; int allow_threads = -1; @@ -1493,6 +1494,7 @@ run_in_subinterp_with_config(PyObject *self, PyObject *args, PyObject *kwargs) PyCompilerFlags cflags = {0}; static char *kwlist[] = {"code", + "use_main_obmalloc", "allow_fork", "allow_exec", "allow_threads", @@ -1500,12 +1502,17 @@ run_in_subinterp_with_config(PyObject *self, PyObject *args, PyObject *kwargs) "check_multi_interp_extensions", NULL}; if (!PyArg_ParseTupleAndKeywords(args, kwargs, - "s$ppppp:run_in_subinterp_with_config", kwlist, - &code, &allow_fork, &allow_exec, + "s$pppppp:run_in_subinterp_with_config", kwlist, + &code, &use_main_obmalloc, + &allow_fork, &allow_exec, &allow_threads, &allow_daemon_threads, &check_multi_interp_extensions)) { return NULL; } + if (use_main_obmalloc < 0) { + PyErr_SetString(PyExc_ValueError, "missing use_main_obmalloc"); + return NULL; + } if (allow_fork < 0) { PyErr_SetString(PyExc_ValueError, "missing allow_fork"); return NULL; @@ -1532,6 +1539,7 @@ run_in_subinterp_with_config(PyObject *self, PyObject *args, PyObject *kwargs) PyThreadState_Swap(NULL); const _PyInterpreterConfig config = { + .use_main_obmalloc = use_main_obmalloc, .allow_fork = allow_fork, .allow_exec = allow_exec, .allow_threads = allow_threads, diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index e80dd30c89dfd0..e2dea7d3cdadbe 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -546,11 +546,19 @@ pycore_init_runtime(_PyRuntimeState *runtime, } -static void +static PyStatus init_interp_settings(PyInterpreterState *interp, const _PyInterpreterConfig *config) { assert(interp->feature_flags == 0); + if (config->use_main_obmalloc) { + interp->feature_flags |= Py_RTFLAGS_USE_MAIN_OBMALLOC; + } + else if (!config->check_multi_interp_extensions) { + return _PyStatus_ERR("per-interpreter obmalloc does not support " + "single-phase init extension modules"); + } + if (config->allow_fork) { interp->feature_flags |= Py_RTFLAGS_FORK; } @@ -569,6 +577,8 @@ init_interp_settings(PyInterpreterState *interp, const _PyInterpreterConfig *con if (config->check_multi_interp_extensions) { interp->feature_flags |= Py_RTFLAGS_MULTI_INTERP_EXTENSIONS; } + + return _PyStatus_OK(); } @@ -621,7 +631,10 @@ pycore_create_interpreter(_PyRuntimeState *runtime, } const _PyInterpreterConfig config = _PyInterpreterConfig_LEGACY_INIT; - init_interp_settings(interp, &config); + status = init_interp_settings(interp, &config); + if (_PyStatus_EXCEPTION(status)) { + return status; + } PyThreadState *tstate = _PyThreadState_New(interp); if (tstate == NULL) { @@ -2031,7 +2044,10 @@ new_interpreter(PyThreadState **tstate_p, const _PyInterpreterConfig *config) goto error; } - init_interp_settings(interp, config); + status = init_interp_settings(interp, config); + if (_PyStatus_EXCEPTION(status)) { + goto error; + } status = init_interp_create_gil(tstate); if (_PyStatus_EXCEPTION(status)) { From 1ae33a05b95b6458a7fbadbc7b31bc98a8b06a09 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 8 Mar 2023 17:37:05 -0700 Subject: [PATCH 08/49] Add a comment about why per-interpreter obmalloc requires multi-phase init extensions. --- Python/pylifecycle.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index e2dea7d3cdadbe..82511d6fda18f7 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -555,6 +555,8 @@ init_interp_settings(PyInterpreterState *interp, const _PyInterpreterConfig *con interp->feature_flags |= Py_RTFLAGS_USE_MAIN_OBMALLOC; } else if (!config->check_multi_interp_extensions) { + /* The reason: PyModuleDef.m_base.m_copy leaks objects between + interpreters. */ return _PyStatus_ERR("per-interpreter obmalloc does not support " "single-phase init extension modules"); } From 5b54d632e862005f0768c05a62d2832a16985d40 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 8 Mar 2023 17:39:44 -0700 Subject: [PATCH 09/49] Add a TODO comment. --- Include/cpython/initconfig.h | 1 + 1 file changed, 1 insertion(+) diff --git a/Include/cpython/initconfig.h b/Include/cpython/initconfig.h index 10c3c3bf502745..f570acd2c31682 100644 --- a/Include/cpython/initconfig.h +++ b/Include/cpython/initconfig.h @@ -244,6 +244,7 @@ PyAPI_FUNC(PyStatus) PyConfig_SetWideStringList(PyConfig *config, /* --- PyInterpreterConfig ------------------------------------ */ typedef struct { + // XXX "allow_object_sharing"? "own_objects"? int use_main_obmalloc; int allow_fork; int allow_exec; From 9f4f8f36a2d3c7ecec42af15c3b731d7f9d1ec13 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 8 Mar 2023 17:30:43 -0700 Subject: [PATCH 10/49] Optionally use the main interpreter's obmalloc state. --- Objects/obmalloc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 29c732ba5d9987..336ef0a0e12d52 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -732,6 +732,9 @@ static inline OMState * get_state(void) { PyInterpreterState *interp = _PyInterpreterState_GET(); + if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { + interp = _PyInterpreterState_Main(); + } return &interp->obmalloc; } From aa102049e999ace80e530bd6653f27234f8d2344 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 9 Mar 2023 12:32:52 -0700 Subject: [PATCH 11/49] Pass use_main_obmalloc to run_in_subinterp() in test_import. --- Lib/test/test_import/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index 96815b3f758a5b..e344138abde4a4 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -1403,6 +1403,9 @@ def test_unwritable_module(self): class SubinterpImportTests(unittest.TestCase): RUN_KWARGS = dict( + # XXX We get crashes if this is False + # (e.g. test.test_import.SubinterpImportTests.test_builtin_compat). + use_main_obmalloc=True, allow_fork=False, allow_exec=False, allow_threads=True, From 69d9a2df4248adbe61cb75bae4fdfe5f6b28f6fe Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 10 Mar 2023 14:40:13 -0700 Subject: [PATCH 12/49] _Py_GetAllocatedBlocks() -> _Py_GetGlobalAllocatedBlocks(). --- Include/internal/pycore_obmalloc.h | 5 +++- Objects/object.c | 2 +- Objects/obmalloc.c | 44 ++++++++++++++++++++++++++++-- Python/sysmodule.c | 4 ++- 4 files changed, 49 insertions(+), 6 deletions(-) diff --git a/Include/internal/pycore_obmalloc.h b/Include/internal/pycore_obmalloc.h index e9a95f4ebc742c..356f3efd5f7f67 100644 --- a/Include/internal/pycore_obmalloc.h +++ b/Include/internal/pycore_obmalloc.h @@ -678,7 +678,10 @@ void _PyObject_VirtualFree(void *, size_t size); /* This function returns the number of allocated memory blocks, regardless of size */ -PyAPI_FUNC(Py_ssize_t) _Py_GetAllocatedBlocks(void); +extern Py_ssize_t _Py_GetGlobalAllocatedBlocks(void); +#define _Py_GetAllocatedBlocks() \ + _Py_GetGlobalAllocatedBlocks() +extern Py_ssize_t _PyInterpreterState_GetAllocatedBlocks(PyInterpreterState *); #ifdef WITH_PYMALLOC diff --git a/Objects/object.c b/Objects/object.c index 38da4d497a96e7..2f603bc27ae001 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -84,7 +84,7 @@ void _PyDebug_PrintTotalRefs(void) { fprintf(stderr, "[%zd refs, %zd blocks]\n", - _Py_GetRefTotal(), _Py_GetAllocatedBlocks()); + _Py_GetRefTotal(), _Py_GetGlobalAllocatedBlocks()); } #endif /* Py_REF_DEBUG */ diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 336ef0a0e12d52..47cae7433fd8e6 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -751,9 +751,9 @@ get_state(void) #define raw_allocated_blocks (state->mgmt.raw_allocated_blocks) Py_ssize_t -_Py_GetAllocatedBlocks(void) +_PyInterpreterState_GetAllocatedBlocks(PyInterpreterState *interp) { - OMState *state = get_state(); + OMState *state = &interp->obmalloc; Py_ssize_t n = raw_allocated_blocks; /* add up allocated blocks for used pools */ @@ -775,6 +775,38 @@ _Py_GetAllocatedBlocks(void) return n; } +Py_ssize_t +_Py_GetGlobalAllocatedBlocks(void) +{ + Py_ssize_t total = 0; + PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(&_PyRuntime); + if (finalizing != NULL) { + assert(finalizing->interp != NULL); + PyInterpreterState *interp = _PyInterpreterState_Main(); + if (interp == NULL) { + /* We are at the very end of runtime finalization. */ + interp = finalizing->interp; + } + else { + assert(interp != NULL); + assert(finalizing->interp == interp); + assert(PyInterpreterState_Head() == interp); + assert(PyInterpreterState_Next(interp) == NULL); + } + total += _PyInterpreterState_GetAllocatedBlocks(interp); + } + else { + HEAD_LOCK(&_PyRuntime); + PyInterpreterState *interp = PyInterpreterState_Head(); + assert(interp != NULL); + for (; interp != NULL; interp = PyInterpreterState_Next(interp)) { + total += _PyInterpreterState_GetAllocatedBlocks(interp); + } + HEAD_UNLOCK(&_PyRuntime); + } + return total; +} + #if WITH_PYMALLOC_RADIX_TREE /*==========================================================================*/ /* radix tree for tracking arena usage. */ @@ -1731,7 +1763,13 @@ _PyObject_Realloc(void *ctx, void *ptr, size_t nbytes) * only be used by extensions that are compiled with pymalloc enabled. */ Py_ssize_t -_Py_GetAllocatedBlocks(void) +_PyInterpreterState_GetAllocatedBlocks(PyInterpreterState *Py_UNUSED(interp)) +{ + return 0; +} + +Py_ssize_t +_Py_GetGlobalAllocatedBlocks(void) { return 0; } diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 207abb964bcac9..7ce93316b22766 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -1865,7 +1865,9 @@ static Py_ssize_t sys_getallocatedblocks_impl(PyObject *module) /*[clinic end generated code: output=f0c4e873f0b6dcf7 input=dab13ee346a0673e]*/ { - return _Py_GetAllocatedBlocks(); + // It might make sense to return the count + // for just the current interpreter. + return _Py_GetGlobalAllocatedBlocks(); } From 25378f87f0a9dc9a28f0d99e343baefa172cc900 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 10 Mar 2023 15:51:12 -0700 Subject: [PATCH 13/49] Errors from _Py_NewInterpreterFromConfig() are no longer fatal. --- Include/cpython/initconfig.h | 1 + Include/cpython/pylifecycle.h | 5 +++-- Include/internal/pycore_initconfig.h | 2 -- Modules/_testcapimodule.c | 6 ++++-- Modules/_xxsubinterpretersmodule.c | 7 +++++-- Python/pylifecycle.c | 19 ++++++++++--------- 6 files changed, 23 insertions(+), 17 deletions(-) diff --git a/Include/cpython/initconfig.h b/Include/cpython/initconfig.h index f570acd2c31682..79c1023baa9a0f 100644 --- a/Include/cpython/initconfig.h +++ b/Include/cpython/initconfig.h @@ -25,6 +25,7 @@ PyAPI_FUNC(PyStatus) PyStatus_Exit(int exitcode); PyAPI_FUNC(int) PyStatus_IsError(PyStatus err); PyAPI_FUNC(int) PyStatus_IsExit(PyStatus err); PyAPI_FUNC(int) PyStatus_Exception(PyStatus err); +PyAPI_FUNC(PyObject *) _PyErr_SetFromPyStatus(PyStatus status); /* --- PyWideStringList ------------------------------------------------ */ diff --git a/Include/cpython/pylifecycle.h b/Include/cpython/pylifecycle.h index e1f83acbffc360..821b169d7a1759 100644 --- a/Include/cpython/pylifecycle.h +++ b/Include/cpython/pylifecycle.h @@ -62,5 +62,6 @@ PyAPI_FUNC(int) _Py_CoerceLegacyLocale(int warn); PyAPI_FUNC(int) _Py_LegacyLocaleDetected(int warn); PyAPI_FUNC(char *) _Py_SetLocaleFromEnv(int category); -PyAPI_FUNC(PyThreadState *) _Py_NewInterpreterFromConfig( - const _PyInterpreterConfig *); +PyAPI_FUNC(PyStatus) _Py_NewInterpreterFromConfig( + PyThreadState **tstate_p, + const _PyInterpreterConfig *config); diff --git a/Include/internal/pycore_initconfig.h b/Include/internal/pycore_initconfig.h index 69f88d7d1d46b8..4cbd14a61d4545 100644 --- a/Include/internal/pycore_initconfig.h +++ b/Include/internal/pycore_initconfig.h @@ -44,8 +44,6 @@ struct pyruntimestate; #define _PyStatus_UPDATE_FUNC(err) \ do { (err).func = _PyStatus_GET_FUNC(); } while (0) -PyObject* _PyErr_SetFromPyStatus(PyStatus status); - /* --- PyWideStringList ------------------------------------------------ */ #define _PyWideStringList_INIT (PyWideStringList){.length = 0, .items = NULL} diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 329e3e92aa4c02..ff13fb30ee771e 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -1546,15 +1546,17 @@ run_in_subinterp_with_config(PyObject *self, PyObject *args, PyObject *kwargs) .allow_daemon_threads = allow_daemon_threads, .check_multi_interp_extensions = check_multi_interp_extensions, }; - substate = _Py_NewInterpreterFromConfig(&config); - if (substate == NULL) { + PyStatus status = _Py_NewInterpreterFromConfig(&substate, &config); + if (PyStatus_Exception(status)) { /* Since no new thread state was created, there is no exception to propagate; raise a fresh one after swapping in the old thread state. */ PyThreadState_Swap(mainstate); + _PyErr_SetFromPyStatus(status); PyErr_SetString(PyExc_RuntimeError, "sub-interpreter creation failed"); return NULL; } + assert(substate != NULL); r = PyRun_SimpleStringFlags(code, &cflags); Py_EndInterpreter(substate); diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index 79dbe3474ba9e8..923160cefdabc9 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -551,15 +551,18 @@ interp_create(PyObject *self, PyObject *args, PyObject *kwds) ? (_PyInterpreterConfig)_PyInterpreterConfig_INIT : (_PyInterpreterConfig)_PyInterpreterConfig_LEGACY_INIT; // XXX Possible GILState issues? - PyThreadState *tstate = _Py_NewInterpreterFromConfig(&config); + PyThreadState *tstate = NULL; + PyStatus status = _Py_NewInterpreterFromConfig(&tstate, &config); PyThreadState_Swap(save_tstate); - if (tstate == NULL) { + if (PyStatus_Exception(status)) { /* Since no new thread state was created, there is no exception to propagate; raise a fresh one after swapping in the old thread state. */ + _PyErr_SetFromPyStatus(status); PyErr_SetString(PyExc_RuntimeError, "interpreter creation failed"); return NULL; } + assert(tstate != NULL); PyInterpreterState *interp = PyThreadState_GetInterpreter(tstate); PyObject *idobj = _PyInterpreterState_GetIDObject(interp); if (idobj == NULL) { diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 82511d6fda18f7..2974787e137213 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2082,22 +2082,23 @@ new_interpreter(PyThreadState **tstate_p, const _PyInterpreterConfig *config) return status; } -PyThreadState * -_Py_NewInterpreterFromConfig(const _PyInterpreterConfig *config) +PyStatus +_Py_NewInterpreterFromConfig(PyThreadState **tstate_p, + const _PyInterpreterConfig *config) { - PyThreadState *tstate = NULL; - PyStatus status = new_interpreter(&tstate, config); - if (_PyStatus_EXCEPTION(status)) { - Py_ExitStatusException(status); - } - return tstate; + return new_interpreter(tstate_p, config); } PyThreadState * Py_NewInterpreter(void) { + PyThreadState *tstate = NULL; const _PyInterpreterConfig config = _PyInterpreterConfig_LEGACY_INIT; - return _Py_NewInterpreterFromConfig(&config); + PyStatus status = _Py_NewInterpreterFromConfig(&tstate, &config); + if (_PyStatus_EXCEPTION(status)) { + Py_ExitStatusException(status); + } + return tstate; } /* Delete an interpreter and its last thread. This requires that the From 1c5b109b85b4df789ef2cfe31523a060b88887fc Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 13 Mar 2023 12:29:23 -0600 Subject: [PATCH 14/49] Chain the exceptions. --- Modules/_testcapimodule.c | 2 ++ Modules/_xxsubinterpretersmodule.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index ff13fb30ee771e..0f33564959eafe 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -1553,7 +1553,9 @@ run_in_subinterp_with_config(PyObject *self, PyObject *args, PyObject *kwargs) state. */ PyThreadState_Swap(mainstate); _PyErr_SetFromPyStatus(status); + PyObject *exc = PyErr_GetRaisedException(); PyErr_SetString(PyExc_RuntimeError, "sub-interpreter creation failed"); + _PyErr_ChainExceptions1(exc); return NULL; } assert(substate != NULL); diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index 923160cefdabc9..117cc3e5bfd459 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -559,7 +559,9 @@ interp_create(PyObject *self, PyObject *args, PyObject *kwds) propagate; raise a fresh one after swapping in the old thread state. */ _PyErr_SetFromPyStatus(status); + PyObject *exc = PyErr_GetRaisedException(); PyErr_SetString(PyExc_RuntimeError, "interpreter creation failed"); + _PyErr_ChainExceptions1(exc); return NULL; } assert(tstate != NULL); From f36426bf8261f9189d3dd78ec905e8ca8a4e3d74 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 10 Mar 2023 15:59:08 -0700 Subject: [PATCH 15/49] Swap out the failed tstate. --- Python/pylifecycle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 2974787e137213..fb3927fd755b79 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2074,10 +2074,10 @@ new_interpreter(PyThreadState **tstate_p, const _PyInterpreterConfig *config) /* Oops, it didn't work. Undo it all. */ PyErr_PrintEx(0); + PyThreadState_Swap(save_tstate); PyThreadState_Clear(tstate); PyThreadState_Delete(tstate); PyInterpreterState_Delete(interp); - PyThreadState_Swap(save_tstate); return status; } From 54b9f09e5da4ef534fc20bd9bd90c444db4f24fe Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 13 Mar 2023 10:10:37 -0600 Subject: [PATCH 16/49] Remaining static builtin types must be fixed. --- Python/pylifecycle.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index fb3927fd755b79..9912845c69c3a4 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -712,6 +712,9 @@ pycore_init_types(PyInterpreterState *interp) return _PyStatus_ERR("failed to initialize an exception type"); } + // XXX init collections module static types (_PyStaticType_InitBuiltin()) + // XXX init IO module static types (_PyStaticType_InitBuiltin()) + status = _PyExc_InitGlobalObjects(interp); if (_PyStatus_EXCEPTION(status)) { return status; @@ -1677,6 +1680,8 @@ finalize_interp_types(PyInterpreterState *interp) _PyFloat_FiniType(interp); _PyLong_FiniTypes(interp); _PyThread_FiniType(interp); + // XXX fini collections module static types (_PyStaticType_Dealloc()) + // XXX fini IO module static types (_PyStaticType_Dealloc()) _PyErr_FiniTypes(interp); _PyTypes_FiniTypes(interp); From 2358a42e03579f2c499010c53b223ef918639f39 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 13 Mar 2023 11:10:44 -0600 Subject: [PATCH 17/49] Add PyInterpreterState.sysdict_copy. --- Include/internal/pycore_interp.h | 1 + Python/pystate.c | 1 + Python/sysmodule.c | 5 +++++ 3 files changed, 7 insertions(+) diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index cdd24dfe11cede..778fc9c82f418e 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -114,6 +114,7 @@ struct _is { PyObject *dict; /* Stores per-interpreter state */ + PyObject *sysdict_copy; PyObject *builtins_copy; // Initialized to _PyEval_EvalFrameDefault(). _PyFrameEvalFunction eval_frame; diff --git a/Python/pystate.c b/Python/pystate.c index 931bba504210ba..3ebb4c4a6f7afe 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -813,6 +813,7 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate) assert(interp->imports.importlib == NULL); assert(interp->imports.import_func == NULL); + Py_CLEAR(interp->sysdict_copy); Py_CLEAR(interp->builtins_copy); Py_CLEAR(interp->dict); #ifdef HAVE_FORK diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 7ce93316b22766..d5082e890167e6 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -3429,6 +3429,11 @@ _PySys_Create(PyThreadState *tstate, PyObject **sysmod_p) } interp->sysdict = Py_NewRef(sysdict); + interp->sysdict_copy = PyDict_Copy(sysdict); + if (interp->sysdict_copy == NULL) { + goto error; + } + if (PyDict_SetItemString(sysdict, "modules", modules) < 0) { goto error; } From b6502e10a040ddfe87226e45a0607ea1e9072488 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 13 Mar 2023 11:19:02 -0600 Subject: [PATCH 18/49] Set m_copy to None for sys and builtins. --- Python/bltinmodule.c | 3 +++ Python/import.c | 35 +++++++++++++++++++++++++++++++---- Python/sysmodule.c | 3 +++ 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index 12ca0ba6c4873c..a8a34620b9bcdf 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -3098,6 +3098,9 @@ _PyBuiltin_Init(PyInterpreterState *interp) } Py_DECREF(debug); + /* m_copy of Py_None means it is copied some other way. */ + builtinsmodule.m_base.m_copy = Py_NewRef(Py_None); + return mod; #undef ADD_TO_ALL #undef SETBUILTIN diff --git a/Python/import.c b/Python/import.c index 57d4eea148810f..8ca9eb425e9da0 100644 --- a/Python/import.c +++ b/Python/import.c @@ -973,6 +973,16 @@ _PyImport_CheckSubinterpIncompatibleExtensionAllowed(const char *name) return 0; } +static inline int +match_mod_name(PyObject *actual, const char *expected) +{ + if (PyUnicode_CompareWithASCIIString(actual, expected) == 0) { + return 1; + } + assert(!PyErr_Occurred()); + return 0; +} + static int fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename) { @@ -996,7 +1006,8 @@ fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename) // when the extension module doesn't support sub-interpreters. // XXX Why special-case the main interpreter? if (_Py_IsMainInterpreter(tstate->interp) || def->m_size == -1) { - if (def->m_size == -1) { + /* m_copy of Py_None means it is copied some other way. */ + if (def->m_size == -1 && def->m_base.m_copy != Py_None) { if (def->m_base.m_copy) { /* Somebody already imported the module, likely under a different name. @@ -1050,18 +1061,34 @@ import_find_extension(PyThreadState *tstate, PyObject *name, PyObject *modules = MODULES(tstate->interp); if (def->m_size == -1) { + PyObject *m_copy = def->m_base.m_copy; /* Module does not support repeated initialization */ - if (def->m_base.m_copy == NULL) + if (m_copy == NULL) { return NULL; + } + else if (m_copy == Py_None) { + if (match_mod_name(name, "sys")) { + m_copy = tstate->interp->sysdict_copy; + } + else if (match_mod_name(name, "builtins")) { + m_copy = tstate->interp->builtins_copy; + } + else { + _PyErr_SetString(tstate, PyExc_ImportError, "missing m_copy"); + return NULL; + } + } + /* m_copy of Py_None means it is copied some other way. */ mod = import_add_module(tstate, name); - if (mod == NULL) + if (mod == NULL) { return NULL; + } mdict = PyModule_GetDict(mod); if (mdict == NULL) { Py_DECREF(mod); return NULL; } - if (PyDict_Update(mdict, def->m_base.m_copy)) { + if (PyDict_Update(mdict, m_copy)) { Py_DECREF(mod); return NULL; } diff --git a/Python/sysmodule.c b/Python/sysmodule.c index d5082e890167e6..1c37f2f2a02a34 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -3423,6 +3423,9 @@ _PySys_Create(PyThreadState *tstate, PyObject **sysmod_p) return _PyStatus_ERR("failed to create a module object"); } + /* m_copy of Py_None means it is copied some other way. */ + sysmodule.m_base.m_copy = Py_NewRef(Py_None); + PyObject *sysdict = PyModule_GetDict(sysmod); if (sysdict == NULL) { goto error; From 678e67bb5b6b79bcddccbd738fcff9b92398b068 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 13 Mar 2023 11:38:54 -0600 Subject: [PATCH 19/49] Add _PyIO_InitTypes(). --- Modules/_io/_iomodule.c | 30 +++++++++++++++++++++++++++--- Python/pylifecycle.c | 13 ++++++++----- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/Modules/_io/_iomodule.c b/Modules/_io/_iomodule.c index d8d836b8382eb1..9f72e5701b345c 100644 --- a/Modules/_io/_iomodule.c +++ b/Modules/_io/_iomodule.c @@ -11,6 +11,7 @@ #include "Python.h" #include "_iomodule.h" #include "pycore_pystate.h" // _PyInterpreterState_GET() +#include "pycore_initconfig.h" // _PyStatus_OK() #ifdef HAVE_SYS_TYPES_H #include @@ -666,12 +667,35 @@ static PyTypeObject* static_types[] = { }; +PyStatus +_PyIO_InitTypes(PyInterpreterState *interp) +{ + if (!_Py_IsMainInterpreter(interp)) { + return _PyStatus_OK(); + } + + for (size_t i=0; i < Py_ARRAY_LENGTH(static_types); i++) { + PyTypeObject *type = static_types[i]; + if (_PyStaticType_InitBuiltin(type) < 0) { + return _PyStatus_ERR("Can't initialize builtin type"); + } + } + + return _PyStatus_OK(); +} + void -_PyIO_Fini(void) +_PyIO_FiniTypes(PyInterpreterState *interp) { + if (!_Py_IsMainInterpreter(interp)) { + return; + } + + // Deallocate types in the reverse order to deallocate subclasses before + // their base classes. for (Py_ssize_t i=Py_ARRAY_LENGTH(static_types) - 1; i >= 0; i--) { - PyTypeObject *exc = static_types[i]; - _PyStaticType_Dealloc(exc); + PyTypeObject *type = static_types[i]; + _PyStaticType_Dealloc(type); } } diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 9912845c69c3a4..3a95dc21407b37 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -31,7 +31,8 @@ #include "pycore_unicodeobject.h" // _PyUnicode_InitTypes() #include "opcode.h" -extern void _PyIO_Fini(void); +extern PyStatus _PyIO_InitTypes(PyInterpreterState *interp); +extern void _PyIO_FiniTypes(PyInterpreterState *interp); #include // setlocale() #include // getenv() @@ -712,8 +713,12 @@ pycore_init_types(PyInterpreterState *interp) return _PyStatus_ERR("failed to initialize an exception type"); } + status = _PyIO_InitTypes(interp); + if (_PyStatus_EXCEPTION(status)) { + return status; + } + // XXX init collections module static types (_PyStaticType_InitBuiltin()) - // XXX init IO module static types (_PyStaticType_InitBuiltin()) status = _PyExc_InitGlobalObjects(interp); if (_PyStatus_EXCEPTION(status)) { @@ -1715,9 +1720,7 @@ finalize_interp_clear(PyThreadState *tstate) /* Clear interpreter state and all thread states */ _PyInterpreterState_Clear(tstate); - if (is_main_interp) { - _PyIO_Fini(); - } + _PyIO_FiniTypes(tstate->interp); /* Clear all loghooks */ /* Both _PySys_Audit function and users still need PyObject, such as tuple. From 69a582923681359a601f87bd829ec4cc02748ec3 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 13 Mar 2023 12:41:02 -0600 Subject: [PATCH 20/49] Fix test_capi. --- Lib/test/test_capi/test_misc.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 40f7d24f487e48..9f75c128652c68 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -1227,10 +1227,12 @@ def test_configured_settings(self): kwlist = [f'allow_{n}' for n in features] kwlist[0] = 'use_main_obmalloc' kwlist[-1] = 'check_multi_interp_extensions' + + # expected to work for config, expected in { (True, True, True, True, True, True): OBMALLOC | FORK | EXEC | THREADS | DAEMON_THREADS | EXTENSIONS, - (False, False, False, False, False, False): 0, + (True, False, False, False, False, False): OBMALLOC, (False, False, False, True, False, True): THREADS | EXTENSIONS, }.items(): kwargs = dict(zip(kwlist, config)) @@ -1253,6 +1255,20 @@ def test_configured_settings(self): self.assertEqual(settings, expected) + # expected to fail + for config in [ + (False, False, False, False, False, False), + ]: + kwargs = dict(zip(kwlist, config)) + with self.subTest(config): + script = textwrap.dedent(f''' + import _testinternalcapi + _testinternalcapi.get_interp_settings() + raise NotImplementedError('unreachable') + ''') + with self.assertRaises(RuntimeError): + support.run_in_subinterp_with_config(script, **kwargs) + @unittest.skipIf(_testsinglephase is None, "test requires _testsinglephase module") @unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()") def test_overridden_setting_extensions_subinterp_check(self): From 3feb408ac01a850ae3149964342fae1497a7f316 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 13 Mar 2023 13:21:45 -0600 Subject: [PATCH 21/49] Avoid allocation for shared exceptions. --- Modules/_xxsubinterpretersmodule.c | 123 ++++++++++++----------------- 1 file changed, 49 insertions(+), 74 deletions(-) diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index 117cc3e5bfd459..9648f080cd756c 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -15,14 +15,14 @@ #define MODULE_NAME "_xxsubinterpreters" -static char * +static const char * _copy_raw_string(PyObject *strobj) { const char *str = PyUnicode_AsUTF8(strobj); if (str == NULL) { return NULL; } - char *copied = PyMem_Malloc(strlen(str)+1); + char *copied = PyMem_RawMalloc(strlen(str)+1); if (copied == NULL) { PyErr_NoMemory(); return NULL; @@ -128,7 +128,7 @@ clear_module_state(module_state *state) /* data-sharing-specific code ***********************************************/ struct _sharednsitem { - char *name; + const char *name; _PyCrossInterpreterData data; }; @@ -152,7 +152,7 @@ static void _sharednsitem_clear(struct _sharednsitem *item) { if (item->name != NULL) { - PyMem_Free(item->name); + PyMem_RawFree((void *)item->name); item->name = NULL; } (void)_release_xid_data(&item->data, 1); @@ -258,96 +258,74 @@ _sharedns_apply(_sharedns *shared, PyObject *ns) // of the exception in the calling interpreter. typedef struct _sharedexception { - char *name; - char *msg; + const char *name; + const char *msg; } _sharedexception; -static _sharedexception * -_sharedexception_new(void) -{ - _sharedexception *err = PyMem_NEW(_sharedexception, 1); - if (err == NULL) { - PyErr_NoMemory(); - return NULL; - } - err->name = NULL; - err->msg = NULL; - return err; -} +static const struct _sharedexception no_exception = { + .name = NULL, + .msg = NULL, +}; static void _sharedexception_clear(_sharedexception *exc) { if (exc->name != NULL) { - PyMem_Free(exc->name); + PyMem_RawFree((void *)exc->name); } if (exc->msg != NULL) { - PyMem_Free(exc->msg); + PyMem_RawFree((void *)exc->msg); } } -static void -_sharedexception_free(_sharedexception *exc) -{ - _sharedexception_clear(exc); - PyMem_Free(exc); -} - -static _sharedexception * -_sharedexception_bind(PyObject *exc) +static const char * +_sharedexception_bind(PyObject *exc, _sharedexception *sharedexc) { assert(exc != NULL); - char *failure = NULL; - - _sharedexception *err = _sharedexception_new(); - if (err == NULL) { - goto finally; - } + const char *failure = NULL; - PyObject *name = PyUnicode_FromFormat("%S", Py_TYPE(exc)); - if (name == NULL) { + PyObject *nameobj = PyUnicode_FromFormat("%S", Py_TYPE(exc)); + if (nameobj == NULL) { failure = "unable to format exception type name"; - goto finally; + goto error; } - err->name = _copy_raw_string(name); - Py_DECREF(name); - if (err->name == NULL) { + sharedexc->name = _copy_raw_string(nameobj); + Py_DECREF(nameobj); + if (sharedexc->name == NULL) { if (PyErr_ExceptionMatches(PyExc_MemoryError)) { failure = "out of memory copying exception type name"; } else { failure = "unable to encode and copy exception type name"; } - goto finally; + goto error; } if (exc != NULL) { - PyObject *msg = PyUnicode_FromFormat("%S", exc); - if (msg == NULL) { + PyObject *msgobj = PyUnicode_FromFormat("%S", exc); + if (msgobj == NULL) { failure = "unable to format exception message"; - goto finally; + goto error; } - err->msg = _copy_raw_string(msg); - Py_DECREF(msg); - if (err->msg == NULL) { + sharedexc->msg = _copy_raw_string(msgobj); + Py_DECREF(msgobj); + if (sharedexc->msg == NULL) { if (PyErr_ExceptionMatches(PyExc_MemoryError)) { failure = "out of memory copying exception message"; } else { failure = "unable to encode and copy exception message"; } - goto finally; + goto error; } } -finally: - if (failure != NULL) { - PyErr_Clear(); - if (err->name != NULL) { - PyMem_Free(err->name); - err->name = NULL; - } - err->msg = failure; - } - return err; + return NULL; + +error: + assert(failure != NULL); + PyErr_Clear(); + _sharedexception_clear(sharedexc); + *sharedexc = no_exception; + return failure; } static void @@ -430,7 +408,7 @@ _ensure_not_running(PyInterpreterState *interp) static int _run_script(PyInterpreterState *interp, const char *codestr, - _sharedns *shared, _sharedexception **exc) + _sharedns *shared, _sharedexception *sharedexc) { PyObject *excval = NULL; PyObject *main_mod = _PyInterpreterState_GetMainModule(interp); @@ -462,22 +440,20 @@ _run_script(PyInterpreterState *interp, const char *codestr, Py_DECREF(result); // We throw away the result. } - *exc = NULL; + *sharedexc = no_exception; return 0; error: excval = PyErr_GetRaisedException(); - _sharedexception *sharedexc = _sharedexception_bind(excval); - Py_XDECREF(excval); - if (sharedexc == NULL) { - fprintf(stderr, "RunFailedError: script raised an uncaught exception"); + const char *failure = _sharedexception_bind(excval, sharedexc); + if (failure != NULL) { + fprintf(stderr, + "RunFailedError: script raised an uncaught exception (%s)", + failure); PyErr_Clear(); - sharedexc = NULL; - } - else { - assert(!PyErr_Occurred()); } - *exc = sharedexc; + Py_XDECREF(excval); + assert(!PyErr_Occurred()); return -1; } @@ -505,7 +481,7 @@ _run_script_in_interpreter(PyObject *mod, PyInterpreterState *interp, } // Run the script. - _sharedexception *exc = NULL; + _sharedexception exc; int result = _run_script(interp, codestr, shared, &exc); // Switch back. @@ -514,10 +490,9 @@ _run_script_in_interpreter(PyObject *mod, PyInterpreterState *interp, } // Propagate any exception out to the caller. - if (exc != NULL) { + if (exc.name != NULL) { assert(state != NULL); - _sharedexception_apply(exc, state->RunFailedError); - _sharedexception_free(exc); + _sharedexception_apply(&exc, state->RunFailedError); } else if (result != 0) { // We were unable to allocate a shared exception. From 05806fcd3c1524628d826df289faf6af2ce7b749 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 13 Mar 2023 14:33:46 -0600 Subject: [PATCH 22/49] Fix the ChannelID tp_name. --- Modules/_xxinterpchannelsmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_xxinterpchannelsmodule.c b/Modules/_xxinterpchannelsmodule.c index a0cd4a2363fb53..fead12c963da26 100644 --- a/Modules/_xxinterpchannelsmodule.c +++ b/Modules/_xxinterpchannelsmodule.c @@ -1806,7 +1806,7 @@ static PyType_Slot ChannelIDType_slots[] = { }; static PyType_Spec ChannelIDType_spec = { - .name = "_xxsubinterpreters.ChannelID", + .name = MODULE_NAME ".ChannelID", .basicsize = sizeof(channelid), .flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_DISALLOW_INSTANTIATION | Py_TPFLAGS_IMMUTABLETYPE), From 4feb2b73682775b2565437b8503c0f0a761d6778 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 29 Mar 2023 09:56:55 -0600 Subject: [PATCH 23/49] Do not include the total from interpreters sharing with main. --- Include/internal/pycore_pystate.h | 7 ++++++ Objects/obmalloc.c | 38 ++++++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index 7046ec8d9adaaf..385d38d09c9bc4 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -33,6 +33,13 @@ _Py_IsMainInterpreter(PyInterpreterState *interp) return (interp == _PyInterpreterState_Main()); } +static inline int +_Py_IsMainInterpreterFinalizing(PyInterpreterState *interp) +{ + return (_PyRuntimeState_GetFinalizing(interp->runtime) != NULL && + interp == &interp->runtime->_main_interpreter); +} + static inline const PyConfig * _Py_GetMainConfig(void) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index d80e09b68f2c61..4a2e6fdbb77a0c 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -727,11 +727,19 @@ static int running_on_valgrind = -1; typedef struct _obmalloc_state OMState; +static inline int +has_own_state(PyInterpreterState *interp) +{ + return (_Py_IsMainInterpreter(interp) || + !(interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) || + _Py_IsMainInterpreterFinalizing(interp)); +} + static inline OMState * get_state(void) { PyInterpreterState *interp = _PyInterpreterState_GET(); - if (interp->feature_flags & Py_RTFLAGS_USE_MAIN_OBMALLOC) { + if (!has_own_state(interp)) { interp = _PyInterpreterState_Main(); } return &interp->obmalloc; @@ -752,6 +760,14 @@ get_state(void) Py_ssize_t _PyInterpreterState_GetAllocatedBlocks(PyInterpreterState *interp) { +#ifdef Py_DEBUG + assert(has_own_state(interp)); +#else + if (!has_own_state(interp)) { + _Py_FatalErrorFunc(__func__, + "the interpreter doesn't have its own allocator"); + } +#endif OMState *state = &interp->obmalloc; Py_ssize_t n = raw_allocated_blocks; @@ -784,24 +800,40 @@ _Py_GetGlobalAllocatedBlocks(void) PyInterpreterState *interp = _PyInterpreterState_Main(); if (interp == NULL) { /* We are at the very end of runtime finalization. */ + assert(PyInterpreterState_Head() == NULL); interp = finalizing->interp; } else { assert(interp != NULL); assert(finalizing->interp == interp); assert(PyInterpreterState_Head() == interp); - assert(PyInterpreterState_Next(interp) == NULL); } + assert(PyInterpreterState_Next(interp) == NULL); total += _PyInterpreterState_GetAllocatedBlocks(interp); } else { HEAD_LOCK(&_PyRuntime); PyInterpreterState *interp = PyInterpreterState_Head(); assert(interp != NULL); +#ifdef Py_DEBUG + int got_main = 0; +#endif for (; interp != NULL; interp = PyInterpreterState_Next(interp)) { - total += _PyInterpreterState_GetAllocatedBlocks(interp); +#ifdef Py_DEBUG + if (_Py_IsMainInterpreter(interp)) { + assert(!got_main); + got_main = 1; + assert(has_own_state(interp)); + } +#endif + if (has_own_state(interp)) { + total += _PyInterpreterState_GetAllocatedBlocks(interp); + } } HEAD_UNLOCK(&_PyRuntime); +#ifdef Py_DEBUG + assert(got_main); +#endif } return total; } From 136ad2f80335647f1ba2951c86cdf6c58cb5d8a3 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 29 Mar 2023 10:10:20 -0600 Subject: [PATCH 24/49] Add _PyRuntime.obmalloc.interpreter_leaks. --- Include/internal/pycore_obmalloc.h | 2 ++ Objects/obmalloc.c | 18 +++++++++++++++--- Python/pystate.c | 5 +++-- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/Include/internal/pycore_obmalloc.h b/Include/internal/pycore_obmalloc.h index 356f3efd5f7f67..ca2a0419b4f038 100644 --- a/Include/internal/pycore_obmalloc.h +++ b/Include/internal/pycore_obmalloc.h @@ -659,6 +659,7 @@ struct _obmalloc_usage { struct _obmalloc_global_state { int dump_debug_stats; + Py_ssize_t interpreter_leaks; }; struct _obmalloc_state { @@ -682,6 +683,7 @@ extern Py_ssize_t _Py_GetGlobalAllocatedBlocks(void); #define _Py_GetAllocatedBlocks() \ _Py_GetGlobalAllocatedBlocks() extern Py_ssize_t _PyInterpreterState_GetAllocatedBlocks(PyInterpreterState *); +extern void _PyInterpreterState_FinalizeAllocatedBlocks(PyInterpreterState *); #ifdef WITH_PYMALLOC diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 4a2e6fdbb77a0c..f0e4b66742c88e 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -790,11 +790,22 @@ _PyInterpreterState_GetAllocatedBlocks(PyInterpreterState *interp) return n; } +void +_PyInterpreterState_FinalizeAllocatedBlocks(PyInterpreterState *interp) +{ + if (has_own_state(interp)) { + Py_ssize_t leaked = _PyInterpreterState_GetAllocatedBlocks(interp); + assert(has_own_state(interp) || leaked == 0); + interp->runtime->obmalloc.interpreter_leaks += leaked; + } +} + Py_ssize_t _Py_GetGlobalAllocatedBlocks(void) { Py_ssize_t total = 0; - PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(&_PyRuntime); + _PyRuntimeState *runtime = &_PyRuntime; + PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(runtime); if (finalizing != NULL) { assert(finalizing->interp != NULL); PyInterpreterState *interp = _PyInterpreterState_Main(); @@ -812,7 +823,7 @@ _Py_GetGlobalAllocatedBlocks(void) total += _PyInterpreterState_GetAllocatedBlocks(interp); } else { - HEAD_LOCK(&_PyRuntime); + HEAD_LOCK(runtime); PyInterpreterState *interp = PyInterpreterState_Head(); assert(interp != NULL); #ifdef Py_DEBUG @@ -830,11 +841,12 @@ _Py_GetGlobalAllocatedBlocks(void) total += _PyInterpreterState_GetAllocatedBlocks(interp); } } - HEAD_UNLOCK(&_PyRuntime); + HEAD_UNLOCK(runtime); #ifdef Py_DEBUG assert(got_main); #endif } + total += runtime->obmalloc.interpreter_leaks; return total; } diff --git a/Python/pystate.c b/Python/pystate.c index 32a8b8b434c7bc..ea4766b9257400 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -912,11 +912,12 @@ PyInterpreterState_Delete(PyInterpreterState *interp) _PyEval_FiniState(&interp->ceval); -#ifdef Py_REF_DEBUG - // XXX This call should be done at the end of clear_interpreter(), + // XXX These two calls should be done at the end of clear_interpreter(), // but currently some objects get decref'ed after that. +#ifdef Py_REF_DEBUG _PyInterpreterState_FinalizeRefTotal(interp); #endif + _PyInterpreterState_FinalizeAllocatedBlocks(interp); HEAD_LOCK(runtime); PyInterpreterState **p; From e19bb37b98eaefa03caf5b5733a286dd0294db73 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 29 Mar 2023 10:27:20 -0600 Subject: [PATCH 25/49] Track leaked blocks across init/fini cycles. --- Include/internal/pycore_pylifecycle.h | 1 + Objects/obmalloc.c | 27 ++++++++++++++++++++++++--- Python/pylifecycle.c | 1 + 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h index a899e848bb8b3c..f96261a650dac7 100644 --- a/Include/internal/pycore_pylifecycle.h +++ b/Include/internal/pycore_pylifecycle.h @@ -64,6 +64,7 @@ extern void _PyAtExit_Fini(PyInterpreterState *interp); extern void _PyThread_FiniType(PyInterpreterState *interp); extern void _Py_Deepfreeze_Fini(void); extern void _PyArg_Fini(void); +extern void _Py_FinalizeAllocatedBlocks(_PyRuntimeState *); extern PyStatus _PyGILState_Init(PyInterpreterState *interp); extern PyStatus _PyGILState_SetTstate(PyThreadState *tstate); diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index f0e4b66742c88e..14445df1d80955 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -800,11 +800,25 @@ _PyInterpreterState_FinalizeAllocatedBlocks(PyInterpreterState *interp) } } -Py_ssize_t -_Py_GetGlobalAllocatedBlocks(void) +static Py_ssize_t get_num_global_allocated_blocks(_PyRuntimeState *); + +/* We preserve the number of blockss leaked during runtime finalization, + so they can be reported if the runtime is initialized again. */ +// XXX We don't lose any information by dropping this, +// so we should consider doing so. +static Py_ssize_t last_final_leaks = 0; + +void +_Py_FinalizeAllocatedBlocks(_PyRuntimeState *runtime) +{ + last_final_leaks = get_num_global_allocated_blocks(runtime); + runtime->object_state.interpreter_leaks = 0; +} + +static Py_ssize_t +get_num_global_allocated_blocks(_PyRuntimeState *runtime) { Py_ssize_t total = 0; - _PyRuntimeState *runtime = &_PyRuntime; PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(runtime); if (finalizing != NULL) { assert(finalizing->interp != NULL); @@ -847,9 +861,16 @@ _Py_GetGlobalAllocatedBlocks(void) #endif } total += runtime->obmalloc.interpreter_leaks; + total += last_final_leaks; return total; } +Py_ssize_t +_Py_GetGlobalAllocatedBlocks(void) +{ + return get_num_global_allocated_blocks(&_PyRuntime); +} + #if WITH_PYMALLOC_RADIX_TREE /*==========================================================================*/ /* radix tree for tracking arena usage. */ diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index cbe23a2dd07416..cd45e057f4bb6c 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1958,6 +1958,7 @@ Py_FinalizeEx(void) } _Py_FinalizeRefTotal(runtime); #endif + _Py_FinalizeAllocatedBlocks(runtime); #ifdef Py_TRACE_REFS /* Display addresses (& refcnts) of all objects still alive. From 6c519972e15da96694c656d772d9a11e94cc3d03 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 29 Mar 2023 17:52:31 -0600 Subject: [PATCH 26/49] Clean up assumptions around runtime fini. --- Objects/obmalloc.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 14445df1d80955..cbe6ad0aed3ac1 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -819,22 +819,21 @@ static Py_ssize_t get_num_global_allocated_blocks(_PyRuntimeState *runtime) { Py_ssize_t total = 0; - PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(runtime); - if (finalizing != NULL) { - assert(finalizing->interp != NULL); + if (_PyRuntimeState_GetFinalizing(runtime) != NULL) { PyInterpreterState *interp = _PyInterpreterState_Main(); if (interp == NULL) { - /* We are at the very end of runtime finalization. */ + /* We are at the very end of runtime finalization. + We can't rely on finalizing->interp since that thread + state is probably already freed, so we don't worry + about it. */ assert(PyInterpreterState_Head() == NULL); - interp = finalizing->interp; } else { assert(interp != NULL); - assert(finalizing->interp == interp); - assert(PyInterpreterState_Head() == interp); + /* It is probably the last interpreter but not necessarily. */ + assert(PyInterpreterState_Next(interp) == NULL); + total += _PyInterpreterState_GetAllocatedBlocks(interp); } - assert(PyInterpreterState_Next(interp) == NULL); - total += _PyInterpreterState_GetAllocatedBlocks(interp); } else { HEAD_LOCK(runtime); From 0ff65ff3cf9638be0178eb64ac93b2c2d6e840ce Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 30 Mar 2023 07:47:12 -0600 Subject: [PATCH 27/49] Add stubs for when WITH_PYMALLOC isn't defined. --- Objects/obmalloc.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index cbe6ad0aed3ac1..65dec5c8f37f2d 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1837,6 +1837,18 @@ _Py_GetGlobalAllocatedBlocks(void) return 0; } +void +_PyInterpreterState_FinalizeAllocatedBlocks(PyInterpreterState *Py_UNUSED(interp)) +{ + return; +} + +void +_Py_FinalizeAllocatedBlocks(_PyRuntimeState *Py_UNUSED(runtime)) +{ + return; +} + #endif /* WITH_PYMALLOC */ From 7db8d4a92c87bb61cb932ba11398608e8b852914 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 31 Mar 2023 09:02:02 -0600 Subject: [PATCH 28/49] Decref the key in the right interpreter in _extensions_cache_set(). --- Python/import.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/import.c b/Python/import.c index a45b3bfaacb252..24249ae4a6ade1 100644 --- a/Python/import.c +++ b/Python/import.c @@ -983,13 +983,13 @@ _extensions_cache_set(PyObject *filename, PyObject *name, PyModuleDef *def) res = 0; finally: + Py_XDECREF(key); if (oldts != NULL) { _PyThreadState_Swap(interp->runtime, oldts); _PyThreadState_UnbindDetached(main_tstate); Py_DECREF(name); Py_DECREF(filename); } - Py_XDECREF(key); extensions_lock_release(); return res; } From 38bee896ff4c0a3f70c02bc45a72854ff6ad2dc3 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 31 Mar 2023 09:04:20 -0600 Subject: [PATCH 29/49] Don't test against sys (for now). --- Lib/test/test_import/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index e344138abde4a4..4fe351375b120b 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -1531,7 +1531,9 @@ def check_incompatible_isolated(self, name): ) def test_builtin_compat(self): - module = 'sys' + # For now we avoid using sys or builtins + # since they still don't implement multi-phase init. + module = '_imp' with self.subTest(f'{module}: not strict'): self.check_compatible_shared(module, strict=False) with self.subTest(f'{module}: strict, shared'): From 375a8f244051cfac2c0752d1dd78d648d57ff7db Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 31 Mar 2023 09:56:17 -0600 Subject: [PATCH 30/49] Clean up SubinterpImportTests. --- Lib/test/test_import/__init__.py | 95 +++++++++++++++++--------------- 1 file changed, 51 insertions(+), 44 deletions(-) diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index 4fe351375b120b..046a478085facc 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -1403,9 +1403,6 @@ def test_unwritable_module(self): class SubinterpImportTests(unittest.TestCase): RUN_KWARGS = dict( - # XXX We get crashes if this is False - # (e.g. test.test_import.SubinterpImportTests.test_builtin_compat). - use_main_obmalloc=True, allow_fork=False, allow_exec=False, allow_threads=True, @@ -1440,10 +1437,10 @@ def import_script(self, name, fd, check_override=None): os.write({fd}, text.encode('utf-8')) ''') - def run_shared(self, name, *, - check_singlephase_setting=False, - check_singlephase_override=None, - ): + def run_here(self, name, *, + check_singlephase_setting=False, + check_singlephase_override=None, + ): """ Try importing the named module in a subinterpreter. @@ -1473,27 +1470,35 @@ def run_shared(self, name, *, self.assertEqual(ret, 0) return os.read(r, 100) - def check_compatible_shared(self, name, *, strict=False): + def check_compatible_here(self, name, *, strict=False): # Verify that the named module may be imported in a subinterpreter. - # (See run_shared() for more info.) - out = self.run_shared(name, check_singlephase_setting=strict) + # (See run_here() for more info.) + out = self.run_here(name, + check_singlephase_setting=strict, + ) self.assertEqual(out, b'okay') - def check_incompatible_shared(self, name): - # Differences from check_compatible_shared(): + def check_incompatible_here(self, name): + # Differences from check_compatible_here(): # * verify that import fails # * "strict" is always True - out = self.run_shared(name, check_singlephase_setting=True) + out = self.run_here(name, + check_singlephase_setting=True, + ) self.assertEqual( out.decode('utf-8'), f'ImportError: module {name} does not support loading in subinterpreters', ) - def check_compatible_isolated(self, name, *, strict=False): - # Differences from check_compatible_shared(): + def check_compatible_fresh(self, name, *, strict=False): + # Differences from check_compatible_here(): # * subinterpreter in a new process # * module has never been imported before in that process # * this tests importing the module for the first time + kwargs = dict( + **self.RUN_KWARGS, + check_multi_interp_extensions=strict, + ) _, out, err = script_helper.assert_python_ok('-c', textwrap.dedent(f''' import _testcapi, sys assert ( @@ -1502,25 +1507,27 @@ def check_compatible_isolated(self, name, *, strict=False): ), repr({name!r}) ret = _testcapi.run_in_subinterp_with_config( {self.import_script(name, "sys.stdout.fileno()")!r}, - **{self.RUN_KWARGS}, - check_multi_interp_extensions={strict}, + **{kwargs}, ) assert ret == 0, ret ''')) self.assertEqual(err, b'') self.assertEqual(out, b'okay') - def check_incompatible_isolated(self, name): - # Differences from check_compatible_isolated(): + def check_incompatible_fresh(self, name): + # Differences from check_compatible_fresh(): # * verify that import fails # * "strict" is always True + kwargs = dict( + **self.RUN_KWARGS, + check_multi_interp_extensions=True, + ) _, out, err = script_helper.assert_python_ok('-c', textwrap.dedent(f''' import _testcapi, sys assert {name!r} not in sys.modules, {name!r} ret = _testcapi.run_in_subinterp_with_config( {self.import_script(name, "sys.stdout.fileno()")!r}, - **{self.RUN_KWARGS}, - check_multi_interp_extensions=True, + **{kwargs}, ) assert ret == 0, ret ''')) @@ -1535,9 +1542,9 @@ def test_builtin_compat(self): # since they still don't implement multi-phase init. module = '_imp' with self.subTest(f'{module}: not strict'): - self.check_compatible_shared(module, strict=False) - with self.subTest(f'{module}: strict, shared'): - self.check_compatible_shared(module, strict=True) + self.check_compatible_here(module, strict=False) + with self.subTest(f'{module}: strict, not fresh'): + self.check_compatible_here(module, strict=True) @cpython_only def test_frozen_compat(self): @@ -1545,47 +1552,47 @@ def test_frozen_compat(self): if __import__(module).__spec__.origin != 'frozen': raise unittest.SkipTest(f'{module} is unexpectedly not frozen') with self.subTest(f'{module}: not strict'): - self.check_compatible_shared(module, strict=False) - with self.subTest(f'{module}: strict, shared'): - self.check_compatible_shared(module, strict=True) + self.check_compatible_here(module, strict=False) + with self.subTest(f'{module}: strict, not fresh'): + self.check_compatible_here(module, strict=True) @unittest.skipIf(_testsinglephase is None, "test requires _testsinglephase module") def test_single_init_extension_compat(self): module = '_testsinglephase' with self.subTest(f'{module}: not strict'): - self.check_compatible_shared(module, strict=False) - with self.subTest(f'{module}: strict, shared'): - self.check_incompatible_shared(module) - with self.subTest(f'{module}: strict, isolated'): - self.check_incompatible_isolated(module) + self.check_compatible_here(module, strict=False) + with self.subTest(f'{module}: strict, not fresh'): + self.check_incompatible_here(module) + with self.subTest(f'{module}: strict, fresh'): + self.check_incompatible_fresh(module) @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") def test_multi_init_extension_compat(self): module = '_testmultiphase' with self.subTest(f'{module}: not strict'): - self.check_compatible_shared(module, strict=False) - with self.subTest(f'{module}: strict, shared'): - self.check_compatible_shared(module, strict=True) - with self.subTest(f'{module}: strict, isolated'): - self.check_compatible_isolated(module, strict=True) + self.check_compatible_here(module, strict=False) + with self.subTest(f'{module}: strict, not fresh'): + self.check_compatible_here(module, strict=True) + with self.subTest(f'{module}: strict, fresh'): + self.check_compatible_fresh(module, strict=True) def test_python_compat(self): module = 'threading' if __import__(module).__spec__.origin == 'frozen': raise unittest.SkipTest(f'{module} is unexpectedly frozen') with self.subTest(f'{module}: not strict'): - self.check_compatible_shared(module, strict=False) - with self.subTest(f'{module}: strict, shared'): - self.check_compatible_shared(module, strict=True) - with self.subTest(f'{module}: strict, isolated'): - self.check_compatible_isolated(module, strict=True) + self.check_compatible_here(module, strict=False) + with self.subTest(f'{module}: strict, not fresh'): + self.check_compatible_here(module, strict=True) + with self.subTest(f'{module}: strict, fresh'): + self.check_compatible_fresh(module, strict=True) @unittest.skipIf(_testsinglephase is None, "test requires _testsinglephase module") def test_singlephase_check_with_setting_and_override(self): module = '_testsinglephase' def check_compatible(setting, override): - out = self.run_shared( + out = self.run_here( module, check_singlephase_setting=setting, check_singlephase_override=override, @@ -1593,7 +1600,7 @@ def check_compatible(setting, override): self.assertEqual(out, b'okay') def check_incompatible(setting, override): - out = self.run_shared( + out = self.run_here( module, check_singlephase_setting=setting, check_singlephase_override=override, From b0a9e11ee23fce841562b44322a20060cbfd1dc2 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 31 Mar 2023 11:01:49 -0600 Subject: [PATCH 31/49] Ensure we are testing against the right type of extension. --- Lib/test/test_import/__init__.py | 54 ++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index 046a478085facc..3ef07203c46c7e 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -4,6 +4,9 @@ import glob import importlib.util from importlib._bootstrap_external import _get_sourcefile +from importlib.machinery import ( + BuiltinImporter, ExtensionFileLoader, FrozenImporter, SourceFileLoader, +) import marshal import os import py_compile @@ -44,6 +47,49 @@ sys.dont_write_bytecode, "test meaningful only when writing bytecode") + +def _require_loader(module, loader, skip): + if isinstance(module, str): + module = __import__(module) + + MODULE_KINDS = { + BuiltinImporter: 'built-in', + ExtensionFileLoader: 'extension', + FrozenImporter: 'frozen', + SourceFileLoader: 'pure Python', + } + + expected = loader + assert isinstance(expected, type), expected + expected = MODULE_KINDS[expected] + + actual = module.__spec__.loader + if not isinstance(actual, type): + actual = type(actual) + actual = MODULE_KINDS[actual] + + if actual != expected: + err = f'expected module to be {expected}, got {module.__spec__}' + if skip: + raise unittest.SkipTest(err) + raise Exception(err) + return module + +def require_builtin(module, *, skip=False): + module = _require_loader(module, BuiltinImporter, skip) + assert module.__spec__.origin == 'built-in', module.__spec__ + +def require_extension(module, *, skip=False): + _require_loader(module, ExtensionFileLoader, skip) + +def require_frozen(module, *, skip=True): + module = _require_loader(module, FrozenImporter, skip) + assert module.__spec__.origin == 'frozen', module.__spec__ + +def require_pure_python(module, *, skip=False): + _require_loader(module, SourceFileLoader, skip) + + def remove_files(name): for f in (name + ".py", name + ".pyc", @@ -1541,6 +1587,7 @@ def test_builtin_compat(self): # For now we avoid using sys or builtins # since they still don't implement multi-phase init. module = '_imp' + require_builtin(module) with self.subTest(f'{module}: not strict'): self.check_compatible_here(module, strict=False) with self.subTest(f'{module}: strict, not fresh'): @@ -1549,6 +1596,7 @@ def test_builtin_compat(self): @cpython_only def test_frozen_compat(self): module = '_frozen_importlib' + require_frozen(module, skip=True) if __import__(module).__spec__.origin != 'frozen': raise unittest.SkipTest(f'{module} is unexpectedly not frozen') with self.subTest(f'{module}: not strict'): @@ -1559,6 +1607,7 @@ def test_frozen_compat(self): @unittest.skipIf(_testsinglephase is None, "test requires _testsinglephase module") def test_single_init_extension_compat(self): module = '_testsinglephase' + require_extension(module) with self.subTest(f'{module}: not strict'): self.check_compatible_here(module, strict=False) with self.subTest(f'{module}: strict, not fresh'): @@ -1569,6 +1618,7 @@ def test_single_init_extension_compat(self): @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") def test_multi_init_extension_compat(self): module = '_testmultiphase' + require_extension(module) with self.subTest(f'{module}: not strict'): self.check_compatible_here(module, strict=False) with self.subTest(f'{module}: strict, not fresh'): @@ -1578,8 +1628,7 @@ def test_multi_init_extension_compat(self): def test_python_compat(self): module = 'threading' - if __import__(module).__spec__.origin == 'frozen': - raise unittest.SkipTest(f'{module} is unexpectedly frozen') + require_pure_python(module) with self.subTest(f'{module}: not strict'): self.check_compatible_here(module, strict=False) with self.subTest(f'{module}: strict, not fresh'): @@ -1590,6 +1639,7 @@ def test_python_compat(self): @unittest.skipIf(_testsinglephase is None, "test requires _testsinglephase module") def test_singlephase_check_with_setting_and_override(self): module = '_testsinglephase' + require_extension(module) def check_compatible(setting, override): out = self.run_here( From 5e5d5d52136dca6fb04777aa299c0f8f0257a87d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 31 Mar 2023 09:30:03 -0600 Subject: [PATCH 32/49] Add a test that uses an isolated interpreter. --- Lib/test/test_import/__init__.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index 3ef07203c46c7e..4da70f61921945 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -1453,7 +1453,12 @@ class SubinterpImportTests(unittest.TestCase): allow_exec=False, allow_threads=True, allow_daemon_threads=False, + # Isolation-related config values aren't included here. ) + ISOLATED = dict( + use_main_obmalloc=False, + ) + NOT_ISOLATED = {k: not v for k, v in ISOLATED.items()} @unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()") def pipe(self): @@ -1486,6 +1491,7 @@ def import_script(self, name, fd, check_override=None): def run_here(self, name, *, check_singlephase_setting=False, check_singlephase_override=None, + isolated=False, ): """ Try importing the named module in a subinterpreter. @@ -1506,6 +1512,7 @@ def run_here(self, name, *, kwargs = dict( **self.RUN_KWARGS, + **(self.ISOLATED if isolated else self.NOT_ISOLATED), check_multi_interp_extensions=check_singlephase_setting, ) @@ -1516,33 +1523,36 @@ def run_here(self, name, *, self.assertEqual(ret, 0) return os.read(r, 100) - def check_compatible_here(self, name, *, strict=False): + def check_compatible_here(self, name, *, strict=False, isolated=False): # Verify that the named module may be imported in a subinterpreter. # (See run_here() for more info.) out = self.run_here(name, check_singlephase_setting=strict, + isolated=isolated, ) self.assertEqual(out, b'okay') - def check_incompatible_here(self, name): + def check_incompatible_here(self, name, *, isolated=False): # Differences from check_compatible_here(): # * verify that import fails # * "strict" is always True out = self.run_here(name, check_singlephase_setting=True, + isolated=isolated, ) self.assertEqual( out.decode('utf-8'), f'ImportError: module {name} does not support loading in subinterpreters', ) - def check_compatible_fresh(self, name, *, strict=False): + def check_compatible_fresh(self, name, *, strict=False, isolated=False): # Differences from check_compatible_here(): # * subinterpreter in a new process # * module has never been imported before in that process # * this tests importing the module for the first time kwargs = dict( **self.RUN_KWARGS, + **(self.ISOLATED if isolated else self.NOT_ISOLATED), check_multi_interp_extensions=strict, ) _, out, err = script_helper.assert_python_ok('-c', textwrap.dedent(f''' @@ -1560,12 +1570,13 @@ def check_compatible_fresh(self, name, *, strict=False): self.assertEqual(err, b'') self.assertEqual(out, b'okay') - def check_incompatible_fresh(self, name): + def check_incompatible_fresh(self, name, *, isolated=False): # Differences from check_compatible_fresh(): # * verify that import fails # * "strict" is always True kwargs = dict( **self.RUN_KWARGS, + **(self.ISOLATED if isolated else self.NOT_ISOLATED), check_multi_interp_extensions=True, ) _, out, err = script_helper.assert_python_ok('-c', textwrap.dedent(f''' @@ -1671,6 +1682,14 @@ def check_incompatible(setting, override): with self.subTest('config: check disabled; override: disabled'): check_compatible(False, -1) + def test_isolated_config(self): + module = 'threading' + require_pure_python(module) + with self.subTest(f'{module}: strict, not fresh'): + self.check_compatible_here(module, strict=True, isolated=True) + with self.subTest(f'{module}: strict, fresh'): + self.check_compatible_fresh(module, strict=True, isolated=True) + if __name__ == '__main__': # Test needs to be a package, so we can do relative imports. From 25809ce4d4b538686320f82a4ebe73ff99104630 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 4 Apr 2023 16:13:53 -0600 Subject: [PATCH 33/49] Fix is_core_module(). --- Python/import.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Python/import.c b/Python/import.c index 24249ae4a6ade1..1db5b9333bbba1 100644 --- a/Python/import.c +++ b/Python/import.c @@ -1110,7 +1110,17 @@ get_core_module_dict(PyInterpreterState *interp, static inline int is_core_module(PyInterpreterState *interp, PyObject *name, PyObject *filename) { - return get_core_module_dict(interp, name, filename) != NULL; + /* This might be called before the core dict copies are in place, + so we can't rely on get_core_module_dict() here. */ + if (filename == name) { + if (PyUnicode_CompareWithASCIIString(name, "sys") == 0) { + return 1; + } + if (PyUnicode_CompareWithASCIIString(name, "builtins") == 0) { + return 1; + } + } + return 0; } static int @@ -1136,6 +1146,8 @@ fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename) // when the extension module doesn't support sub-interpreters. if (def->m_size == -1) { if (!is_core_module(tstate->interp, name, filename)) { + assert(PyUnicode_CompareWithASCIIString(name, "sys") != 0); + assert(PyUnicode_CompareWithASCIIString(name, "builtins") != 0); if (def->m_base.m_copy) { /* Somebody already imported the module, likely under a different name. From 43a836bfe137fa74fde46456b6f9ea34959429ba Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 4 Apr 2023 16:44:32 -0600 Subject: [PATCH 34/49] Ignore last_final_leaks. --- Tools/c-analyzer/cpython/ignored.tsv | 1 + 1 file changed, 1 insertion(+) diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index a8ba88efc732fb..7a5d7d45f5184b 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -309,6 +309,7 @@ Objects/obmalloc.c - _PyMem - Objects/obmalloc.c - _PyMem_Debug - Objects/obmalloc.c - _PyMem_Raw - Objects/obmalloc.c - _PyObject - +Objects/obmalloc.c - last_final_leaks - Objects/obmalloc.c - usedpools - Objects/typeobject.c - name_op - Objects/typeobject.c - slotdefs - From 1841b55f8900a16f111c37984e41d26c4096a4b4 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 4 Apr 2023 17:13:24 -0600 Subject: [PATCH 35/49] Fix a typo. --- Objects/obmalloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 65dec5c8f37f2d..de62aeb04461fa 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -812,7 +812,7 @@ void _Py_FinalizeAllocatedBlocks(_PyRuntimeState *runtime) { last_final_leaks = get_num_global_allocated_blocks(runtime); - runtime->object_state.interpreter_leaks = 0; + runtime->obmalloc.interpreter_leaks = 0; } static Py_ssize_t From 0091e4839cba4d5b96710d93d69b8dd441bd009f Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 10:13:49 -0600 Subject: [PATCH 36/49] Add a note about global state owned by the module. --- Modules/_xxinterpchannelsmodule.c | 62 +++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/Modules/_xxinterpchannelsmodule.c b/Modules/_xxinterpchannelsmodule.c index fead12c963da26..90136d9bc85743 100644 --- a/Modules/_xxinterpchannelsmodule.c +++ b/Modules/_xxinterpchannelsmodule.c @@ -10,6 +10,68 @@ #include "pycore_interpreteridobject.h" +/* +This module has the following process-global state: + +_globals (static struct globals): + module_count (int) + channels (struct _channels): + numopen (int64_t) + next_id; (int64_t) + mutex (PyThread_type_lock) + head (linked list of struct _channelref *): + id (int64_t) + objcount (Py_ssize_t) + next (struct _channelref *): + ... + chan (struct _channel *): + open (int) + mutex (PyThread_type_lock) + closing (struct _channel_closing *): + ref (struct _channelref *): + ... + ends (struct _channelends *): + numsendopen (int64_t) + numrecvopen (int64_t) + send (struct _channelend *): + interp (int64_t) + open (int) + next (struct _channelend *) + recv (struct _channelend *): + ... + queue (struct _channelqueue *): + count (int64_t) + first (struct _channelitem *): + next (struct _channelitem *): + ... + data (_PyCrossInterpreterData *): + data (void *) + obj (PyObject *) + interp (int64_t) + new_object (xid_newobjectfunc) + free (xid_freefunc) + last (struct _channelitem *): + ... + +The above state includes the following allocations by the module: + +* 1 top-level mutex (to protect the rest of the state) +* for each channel: + * 1 struct _channelref + * 1 struct _channel + * 0-1 struct _channel_closing + * 1 struct _channelends + * 2 struct _channelend + * 1 struct _channelqueue +* for each item in each channel: + * 1 struct _channelitem + * 1 _PyCrossInterpreterData + +The only objects in that global state are the references held by each +channel's queue, which are safely managed via the _PyCrossInterpreterData_*() +API.. The module does not create any objects that are shared globally. +*/ + #define MODULE_NAME "_xxinterpchannels" From 9f74f7bde326f1cdb278a66c3aee48f4d8719211 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 10:32:08 -0600 Subject: [PATCH 37/49] Factor out GLOBAL_MALLOC() and GLOBAL_FREE(). --- Modules/_xxinterpchannelsmodule.c | 53 ++++++++++++++++++------------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/Modules/_xxinterpchannelsmodule.c b/Modules/_xxinterpchannelsmodule.c index 90136d9bc85743..ecf8604f8d7623 100644 --- a/Modules/_xxinterpchannelsmodule.c +++ b/Modules/_xxinterpchannelsmodule.c @@ -75,6 +75,12 @@ API.. The module does not create any objects that are shared globally. #define MODULE_NAME "_xxinterpchannels" +#define GLOBAL_MALLOC(TYPE) \ + PyMem_NEW(TYPE, 1) +#define GLOBAL_FREE(VAR) \ + PyMem_Free(VAR) + + static PyInterpreterState * _get_current_interp(void) { @@ -363,7 +369,7 @@ typedef struct _channelitem { static _channelitem * _channelitem_new(void) { - _channelitem *item = PyMem_NEW(_channelitem, 1); + _channelitem *item = GLOBAL_MALLOC(_channelitem); if (item == NULL) { PyErr_NoMemory(); return NULL; @@ -378,7 +384,8 @@ _channelitem_clear(_channelitem *item) { if (item->data != NULL) { (void)_release_xid_data(item->data, 1); - PyMem_Free(item->data); + // It was allocated in _channel_send(). + GLOBAL_FREE(item->data); item->data = NULL; } item->next = NULL; @@ -388,7 +395,7 @@ static void _channelitem_free(_channelitem *item) { _channelitem_clear(item); - PyMem_Free(item); + GLOBAL_FREE(item); } static void @@ -419,7 +426,7 @@ typedef struct _channelqueue { static _channelqueue * _channelqueue_new(void) { - _channelqueue *queue = PyMem_NEW(_channelqueue, 1); + _channelqueue *queue = GLOBAL_MALLOC(_channelqueue); if (queue == NULL) { PyErr_NoMemory(); return NULL; @@ -443,7 +450,7 @@ static void _channelqueue_free(_channelqueue *queue) { _channelqueue_clear(queue); - PyMem_Free(queue); + GLOBAL_FREE(queue); } static int @@ -495,7 +502,7 @@ typedef struct _channelend { static _channelend * _channelend_new(int64_t interp) { - _channelend *end = PyMem_NEW(_channelend, 1); + _channelend *end = GLOBAL_MALLOC(_channelend); if (end == NULL) { PyErr_NoMemory(); return NULL; @@ -509,7 +516,7 @@ _channelend_new(int64_t interp) static void _channelend_free(_channelend *end) { - PyMem_Free(end); + GLOBAL_FREE(end); } static void @@ -554,7 +561,7 @@ typedef struct _channelassociations { static _channelends * _channelends_new(void) { - _channelends *ends = PyMem_NEW(_channelends, 1); + _channelends *ends = GLOBAL_MALLOC(_channelends); if (ends== NULL) { return NULL; } @@ -581,7 +588,7 @@ static void _channelends_free(_channelends *ends) { _channelends_clear(ends); - PyMem_Free(ends); + GLOBAL_FREE(ends); } static _channelend * @@ -722,20 +729,20 @@ typedef struct _channel { static _PyChannelState * _channel_new(PyThread_type_lock mutex) { - _PyChannelState *chan = PyMem_NEW(_PyChannelState, 1); + _PyChannelState *chan = GLOBAL_MALLOC(_PyChannelState); if (chan == NULL) { return NULL; } chan->mutex = mutex; chan->queue = _channelqueue_new(); if (chan->queue == NULL) { - PyMem_Free(chan); + GLOBAL_FREE(chan); return NULL; } chan->ends = _channelends_new(); if (chan->ends == NULL) { _channelqueue_free(chan->queue); - PyMem_Free(chan); + GLOBAL_FREE(chan); return NULL; } chan->open = 1; @@ -753,7 +760,7 @@ _channel_free(_PyChannelState *chan) PyThread_release_lock(chan->mutex); PyThread_free_lock(chan->mutex); - PyMem_Free(chan); + GLOBAL_FREE(chan); } static int @@ -876,7 +883,7 @@ typedef struct _channelref { static _channelref * _channelref_new(int64_t id, _PyChannelState *chan) { - _channelref *ref = PyMem_NEW(_channelref, 1); + _channelref *ref = GLOBAL_MALLOC(_channelref); if (ref == NULL) { return NULL; } @@ -903,7 +910,7 @@ _channelref_free(_channelref *ref) _channel_clear_closing(ref->chan); } //_channelref_clear(ref); - PyMem_Free(ref); + GLOBAL_FREE(ref); } static _channelref * @@ -1225,7 +1232,7 @@ _channel_set_closing(struct _channelref *ref, PyThread_type_lock mutex) { res = ERR_CHANNEL_CLOSED; goto done; } - chan->closing = PyMem_NEW(struct _channel_closing, 1); + chan->closing = GLOBAL_MALLOC(struct _channel_closing); if (chan->closing == NULL) { goto done; } @@ -1241,7 +1248,7 @@ static void _channel_clear_closing(struct _channel *chan) { PyThread_acquire_lock(chan->mutex, WAIT_LOCK); if (chan->closing != NULL) { - PyMem_Free(chan->closing); + GLOBAL_FREE(chan->closing); chan->closing = NULL; } PyThread_release_lock(chan->mutex); @@ -1319,14 +1326,14 @@ _channel_send(_channels *channels, int64_t id, PyObject *obj) } // Convert the object to cross-interpreter data. - _PyCrossInterpreterData *data = PyMem_NEW(_PyCrossInterpreterData, 1); + _PyCrossInterpreterData *data = GLOBAL_MALLOC(_PyCrossInterpreterData); if (data == NULL) { PyThread_release_lock(mutex); return -1; } if (_PyObject_GetCrossInterpreterData(obj, data) != 0) { PyThread_release_lock(mutex); - PyMem_Free(data); + GLOBAL_FREE(data); return -1; } @@ -1336,7 +1343,7 @@ _channel_send(_channels *channels, int64_t id, PyObject *obj) if (res != 0) { // We may chain an exception here: (void)_release_xid_data(data, 0); - PyMem_Free(data); + GLOBAL_FREE(data); return res; } @@ -1385,11 +1392,13 @@ _channel_recv(_channels *channels, int64_t id, PyObject **res) if (obj == NULL) { assert(PyErr_Occurred()); (void)_release_xid_data(data, 1); - PyMem_Free(data); + // It was allocated in _channel_send(). + GLOBAL_FREE(data); return -1; } int release_res = _release_xid_data(data, 0); - PyMem_Free(data); + // It was allocated in _channel_send(). + GLOBAL_FREE(data); if (release_res < 0) { // The source interpreter has been destroyed already. assert(PyErr_Occurred()); From 10c35890e88a5390ea367d57bfcb1018090fe543 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 10:34:17 -0600 Subject: [PATCH 38/49] Switch to the raw allocator. --- Modules/_xxinterpchannelsmodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_xxinterpchannelsmodule.c b/Modules/_xxinterpchannelsmodule.c index ecf8604f8d7623..ef1cdcab952271 100644 --- a/Modules/_xxinterpchannelsmodule.c +++ b/Modules/_xxinterpchannelsmodule.c @@ -76,9 +76,9 @@ API.. The module does not create any objects that are shared globally. #define GLOBAL_MALLOC(TYPE) \ - PyMem_NEW(TYPE, 1) + PyMem_RawMalloc(sizeof(TYPE)) #define GLOBAL_FREE(VAR) \ - PyMem_Free(VAR) + PyMem_RawFree(VAR) static PyInterpreterState * From 593430b7ccfdfd0b7799e9c6b6b5360405769a50 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 11:28:50 -0600 Subject: [PATCH 39/49] Use the raw allocator for _PyCrossInterpreterData_InitWithSize(). --- Python/pystate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/pystate.c b/Python/pystate.c index d331977eaa3826..e3f611e56145e7 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -2286,11 +2286,11 @@ _PyCrossInterpreterData_InitWithSize(_PyCrossInterpreterData *data, // where it was allocated, so the interpreter is required. assert(interp != NULL); _PyCrossInterpreterData_Init(data, interp, NULL, obj, new_object); - data->data = PyMem_Malloc(size); + data->data = PyMem_RawMalloc(size); if (data->data == NULL) { return -1; } - data->free = PyMem_Free; + data->free = PyMem_RawFree; return 0; } From f5ae7107974ccd6cb942093f9a878635e2f7d327 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 14:33:29 -0600 Subject: [PATCH 40/49] atexit_callback -> atexit_py_callback. --- Include/internal/pycore_interp.h | 4 ++-- Modules/atexitmodule.c | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 1f2c0db2eb5f27..ce0325a9e02d13 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -37,10 +37,10 @@ typedef struct { PyObject *func; PyObject *args; PyObject *kwargs; -} atexit_callback; +} atexit_py_callback; struct atexit_state { - atexit_callback **callbacks; + atexit_py_callback **callbacks; int ncallbacks; int callback_len; }; diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index a1c511e09d704e..00b186fc15015e 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -25,7 +25,7 @@ get_atexit_state(void) static void atexit_delete_cb(struct atexit_state *state, int i) { - atexit_callback *cb = state->callbacks[i]; + atexit_py_callback *cb = state->callbacks[i]; state->callbacks[i] = NULL; Py_DECREF(cb->func); @@ -39,7 +39,7 @@ atexit_delete_cb(struct atexit_state *state, int i) static void atexit_cleanup(struct atexit_state *state) { - atexit_callback *cb; + atexit_py_callback *cb; for (int i = 0; i < state->ncallbacks; i++) { cb = state->callbacks[i]; if (cb == NULL) @@ -60,7 +60,7 @@ _PyAtExit_Init(PyInterpreterState *interp) state->callback_len = 32; state->ncallbacks = 0; - state->callbacks = PyMem_New(atexit_callback*, state->callback_len); + state->callbacks = PyMem_New(atexit_py_callback*, state->callback_len); if (state->callbacks == NULL) { return _PyStatus_NO_MEMORY(); } @@ -88,7 +88,7 @@ atexit_callfuncs(struct atexit_state *state) } for (int i = state->ncallbacks - 1; i >= 0; i--) { - atexit_callback *cb = state->callbacks[i]; + atexit_py_callback *cb = state->callbacks[i]; if (cb == NULL) { continue; } @@ -152,17 +152,17 @@ atexit_register(PyObject *module, PyObject *args, PyObject *kwargs) struct atexit_state *state = get_atexit_state(); if (state->ncallbacks >= state->callback_len) { - atexit_callback **r; + atexit_py_callback **r; state->callback_len += 16; - size_t size = sizeof(atexit_callback*) * (size_t)state->callback_len; - r = (atexit_callback**)PyMem_Realloc(state->callbacks, size); + size_t size = sizeof(atexit_py_callback*) * (size_t)state->callback_len; + r = (atexit_py_callback**)PyMem_Realloc(state->callbacks, size); if (r == NULL) { return PyErr_NoMemory(); } state->callbacks = r; } - atexit_callback *callback = PyMem_Malloc(sizeof(atexit_callback)); + atexit_py_callback *callback = PyMem_Malloc(sizeof(atexit_py_callback)); if (callback == NULL) { return PyErr_NoMemory(); } @@ -233,7 +233,7 @@ atexit_unregister(PyObject *module, PyObject *func) struct atexit_state *state = get_atexit_state(); for (int i = 0; i < state->ncallbacks; i++) { - atexit_callback *cb = state->callbacks[i]; + atexit_py_callback *cb = state->callbacks[i]; if (cb == NULL) { continue; } From e6d4776ad1861b05bf3b16ac74151ee33a583c83 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 14:50:44 -0600 Subject: [PATCH 41/49] Add pycore_atexit.h. --- Include/internal/pycore_atexit.h | 39 ++++++++++++++++++++++++++++++ Include/internal/pycore_interp.h | 17 ++----------- Include/internal/pycore_runtime.h | 5 ++-- Makefile.pre.in | 1 + Modules/atexitmodule.c | 1 + PCbuild/pythoncore.vcxproj | 1 + PCbuild/pythoncore.vcxproj.filters | 3 +++ Python/pylifecycle.c | 14 +++++------ 8 files changed, 56 insertions(+), 25 deletions(-) create mode 100644 Include/internal/pycore_atexit.h diff --git a/Include/internal/pycore_atexit.h b/Include/internal/pycore_atexit.h new file mode 100644 index 00000000000000..1f1c3a1c3ce0fa --- /dev/null +++ b/Include/internal/pycore_atexit.h @@ -0,0 +1,39 @@ +#ifndef Py_INTERNAL_ATEXIT_H +#define Py_INTERNAL_ATEXIT_H +#ifdef __cplusplus +extern "C" { +#endif + +#ifndef Py_BUILD_CORE +# error "this header requires Py_BUILD_CORE define" +#endif + + +typedef void (*atexit_callbackfunc)(void); + + +typedef struct { + PyObject *func; + PyObject *args; + PyObject *kwargs; +} atexit_py_callback; + +struct atexit_state { + atexit_py_callback **callbacks; + int ncallbacks; + int callback_len; +}; + + + +struct _atexit_runtime_state { +#define NEXITFUNCS 32 + atexit_callbackfunc callbacks[NEXITFUNCS]; + int ncallbacks; +}; + + +#ifdef __cplusplus +} +#endif +#endif /* !Py_INTERNAL_ATEXIT_H */ diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index ce0325a9e02d13..d64a68cd2da54e 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -10,8 +10,9 @@ extern "C" { #include -#include "pycore_atomic.h" // _Py_atomic_address #include "pycore_ast_state.h" // struct ast_state +#include "pycore_atexit.h" // struct atexit_state +#include "pycore_atomic.h" // _Py_atomic_address #include "pycore_ceval_state.h" // struct _ceval_state #include "pycore_code.h" // struct callable_cache #include "pycore_context.h" // struct _Py_context_state @@ -32,20 +33,6 @@ extern "C" { #include "pycore_warnings.h" // struct _warnings_runtime_state -// atexit state -typedef struct { - PyObject *func; - PyObject *args; - PyObject *kwargs; -} atexit_py_callback; - -struct atexit_state { - atexit_py_callback **callbacks; - int ncallbacks; - int callback_len; -}; - - struct _Py_long_state { int max_str_digits; }; diff --git a/Include/internal/pycore_runtime.h b/Include/internal/pycore_runtime.h index 8877b282bc38de..3ebe49926edda6 100644 --- a/Include/internal/pycore_runtime.h +++ b/Include/internal/pycore_runtime.h @@ -8,6 +8,7 @@ extern "C" { # error "this header requires Py_BUILD_CORE define" #endif +#include "pycore_atexit.h" // struct atexit_runtime_state #include "pycore_atomic.h" /* _Py_atomic_address */ #include "pycore_ceval_state.h" // struct _ceval_runtime_state #include "pycore_floatobject.h" // struct _Py_float_runtime_state @@ -131,9 +132,7 @@ typedef struct pyruntimestate { struct _parser_runtime_state parser; -#define NEXITFUNCS 32 - void (*exitfuncs[NEXITFUNCS])(void); - int nexitfuncs; + struct _atexit_runtime_state atexit; struct _import_runtime_state imports; struct _ceval_runtime_state ceval; diff --git a/Makefile.pre.in b/Makefile.pre.in index 9fdbd8db19bb33..1c1bddcad82475 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -1660,6 +1660,7 @@ PYTHON_HEADERS= \ $(srcdir)/Include/internal/pycore_asdl.h \ $(srcdir)/Include/internal/pycore_ast.h \ $(srcdir)/Include/internal/pycore_ast_state.h \ + $(srcdir)/Include/internal/pycore_atexit.h \ $(srcdir)/Include/internal/pycore_atomic.h \ $(srcdir)/Include/internal/pycore_atomic_funcs.h \ $(srcdir)/Include/internal/pycore_bitutils.h \ diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index 00b186fc15015e..e8e964f75155ae 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -7,6 +7,7 @@ */ #include "Python.h" +#include "pycore_atexit.h" #include "pycore_initconfig.h" // _PyStatus_NO_MEMORY #include "pycore_interp.h" // PyInterpreterState.atexit #include "pycore_pystate.h" // _PyInterpreterState_GET diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj index 8fab600334160d..29f32db579fa40 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -197,6 +197,7 @@ + diff --git a/PCbuild/pythoncore.vcxproj.filters b/PCbuild/pythoncore.vcxproj.filters index 6c5d8dd89f5bc7..6a622fd93930ad 100644 --- a/PCbuild/pythoncore.vcxproj.filters +++ b/PCbuild/pythoncore.vcxproj.filters @@ -498,6 +498,9 @@ Include\internal + + Include\internal + Include\internal diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 8110d94ba17526..d6627bc6b7e86b 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2937,23 +2937,23 @@ wait_for_thread_shutdown(PyThreadState *tstate) Py_DECREF(threading); } -#define NEXITFUNCS 32 int Py_AtExit(void (*func)(void)) { - if (_PyRuntime.nexitfuncs >= NEXITFUNCS) + if (_PyRuntime.atexit.ncallbacks >= NEXITFUNCS) return -1; - _PyRuntime.exitfuncs[_PyRuntime.nexitfuncs++] = func; + _PyRuntime.atexit.callbacks[_PyRuntime.atexit.ncallbacks++] = func; return 0; } static void call_ll_exitfuncs(_PyRuntimeState *runtime) { - while (runtime->nexitfuncs > 0) { + struct _atexit_runtime_state *state = &runtime->atexit; + while (state->ncallbacks > 0) { /* pop last function from the list */ - runtime->nexitfuncs--; - void (*exitfunc)(void) = runtime->exitfuncs[runtime->nexitfuncs]; - runtime->exitfuncs[runtime->nexitfuncs] = NULL; + state->ncallbacks--; + atexit_callbackfunc exitfunc = state->callbacks[state->ncallbacks]; + state->callbacks[state->ncallbacks] = NULL; exitfunc(); } From c719f0214891d0a0dceee30d27a00cf9f7719694 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 15:55:30 -0600 Subject: [PATCH 42/49] Add _Py_AtExit(). --- Include/internal/pycore_atexit.h | 33 ++++++++++++++++++++++------ Modules/_testinternalcapi.c | 33 ++++++++++++++++++++++++++++ Modules/atexitmodule.c | 37 ++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 7 deletions(-) diff --git a/Include/internal/pycore_atexit.h b/Include/internal/pycore_atexit.h index 1f1c3a1c3ce0fa..994d9c8148a3e3 100644 --- a/Include/internal/pycore_atexit.h +++ b/Include/internal/pycore_atexit.h @@ -9,8 +9,29 @@ extern "C" { #endif +//############### +// runtime atexit + typedef void (*atexit_callbackfunc)(void); +struct _atexit_runtime_state { +#define NEXITFUNCS 32 + atexit_callbackfunc callbacks[NEXITFUNCS]; + int ncallbacks; +}; + + +//################### +// interpreter atexit + +typedef void (*atexit_datacallbackfunc)(void *); + +struct atexit_callback; +typedef struct atexit_callback { + atexit_datacallbackfunc func; + void *data; + struct atexit_callback *next; +} atexit_callback; typedef struct { PyObject *func; @@ -22,16 +43,14 @@ struct atexit_state { atexit_py_callback **callbacks; int ncallbacks; int callback_len; -}; - - -struct _atexit_runtime_state { -#define NEXITFUNCS 32 - atexit_callbackfunc callbacks[NEXITFUNCS]; - int ncallbacks; + atexit_callback *ll_callbacks; + atexit_callback *last_ll_callback; }; +PyAPI_FUNC(int) _Py_AtExit( + PyInterpreterState *, atexit_datacallbackfunc, void *); + #ifdef __cplusplus } diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 632fac2de0c419..47a41c8eedf185 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -12,6 +12,7 @@ #define PY_SSIZE_T_CLEAN #include "Python.h" +#include "pycore_atexit.h" // _Py_AtExit() #include "pycore_atomic_funcs.h" // _Py_atomic_int_get() #include "pycore_bitutils.h" // _Py_bswap32() #include "pycore_compile.h" // _PyCompile_CodeGen, _PyCompile_OptimizeCfg @@ -685,6 +686,37 @@ clear_extension(PyObject *self, PyObject *args) } +struct atexit_data { + int called; +}; + +static void +callback(void *data) +{ + ((struct atexit_data *)data)->called += 1; +} + +static PyObject * +test_atexit(PyObject *self, PyObject *Py_UNUSED(args)) +{ + PyThreadState *oldts = PyThreadState_Swap(NULL); + PyThreadState *tstate = Py_NewInterpreter(); + + struct atexit_data data = {0}; + int res = _Py_AtExit(tstate->interp, callback, (void *)&data); + Py_EndInterpreter(tstate); + PyThreadState_Swap(oldts); + if (res < 0) { + return NULL; + } + if (data.called == 0) { + PyErr_SetString(PyExc_RuntimeError, "atexit callback not called"); + return NULL; + } + Py_RETURN_NONE; +} + + static PyMethodDef module_functions[] = { {"get_configs", get_configs, METH_NOARGS}, {"get_recursion_depth", get_recursion_depth, METH_NOARGS}, @@ -707,6 +739,7 @@ static PyMethodDef module_functions[] = { _TESTINTERNALCAPI_OPTIMIZE_CFG_METHODDEF {"get_interp_settings", get_interp_settings, METH_VARARGS, NULL}, {"clear_extension", clear_extension, METH_VARARGS, NULL}, + {"test_atexit", test_atexit, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index e8e964f75155ae..5f78eeb87acdcd 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -23,6 +23,31 @@ get_atexit_state(void) } +int +_Py_AtExit(PyInterpreterState *interp, + atexit_datacallbackfunc func, void *data) +{ + atexit_callback *callback = PyMem_Malloc(sizeof(atexit_callback)); + if (callback == NULL) { + PyErr_NoMemory(); + return -1; + } + callback->func = func; + callback->data = data; + callback->next = NULL; + + struct atexit_state *state = &interp->atexit; + if (state->ll_callbacks == NULL) { + state->ll_callbacks = callback; + state->last_ll_callback = callback; + } + else { + state->last_ll_callback->next = callback; + } + return 0; +} + + static void atexit_delete_cb(struct atexit_state *state, int i) { @@ -76,6 +101,18 @@ _PyAtExit_Fini(PyInterpreterState *interp) atexit_cleanup(state); PyMem_Free(state->callbacks); state->callbacks = NULL; + + atexit_callback *next = state->ll_callbacks; + state->ll_callbacks = NULL; + while (next != NULL) { + atexit_callback *callback = next; + next = callback->next; + atexit_datacallbackfunc exitfunc = callback->func; + void *data = callback->data; + // It was allocated in _PyAtExit_AddCallback(). + PyMem_Free(callback); + exitfunc(data); + } } From 47c302d459fff4d16ebf1f72fb1ae7c835d27fc9 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 16:04:20 -0600 Subject: [PATCH 43/49] Add a TODO comment. --- Include/internal/pycore_atexit.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_atexit.h b/Include/internal/pycore_atexit.h index 994d9c8148a3e3..8b569422bc44bb 100644 --- a/Include/internal/pycore_atexit.h +++ b/Include/internal/pycore_atexit.h @@ -40,12 +40,15 @@ typedef struct { } atexit_py_callback; struct atexit_state { + atexit_callback *ll_callbacks; + atexit_callback *last_ll_callback; + + // XXX The rest of the state could be moved to the atexit module state + // and a low-level callback added for it during module exec. + // For the moment we leave it here. atexit_py_callback **callbacks; int ncallbacks; int callback_len; - - atexit_callback *ll_callbacks; - atexit_callback *last_ll_callback; }; PyAPI_FUNC(int) _Py_AtExit( From aaeaaa6b1d8aeed439f598298291b4650aa4224a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 16:13:24 -0600 Subject: [PATCH 44/49] Move _Py_AtExit() to the public API. --- Include/cpython/pylifecycle.h | 4 ++++ Include/internal/pycore_atexit.h | 5 ----- Modules/_testcapimodule.c | 32 +++++++++++++++++++++++++++++++ Modules/_testinternalcapi.c | 33 -------------------------------- 4 files changed, 36 insertions(+), 38 deletions(-) diff --git a/Include/cpython/pylifecycle.h b/Include/cpython/pylifecycle.h index 821b169d7a1759..79d55711319e55 100644 --- a/Include/cpython/pylifecycle.h +++ b/Include/cpython/pylifecycle.h @@ -65,3 +65,7 @@ PyAPI_FUNC(char *) _Py_SetLocaleFromEnv(int category); PyAPI_FUNC(PyStatus) _Py_NewInterpreterFromConfig( PyThreadState **tstate_p, const _PyInterpreterConfig *config); + +typedef void (*atexit_datacallbackfunc)(void *); +PyAPI_FUNC(int) _Py_AtExit( + PyInterpreterState *, atexit_datacallbackfunc, void *); diff --git a/Include/internal/pycore_atexit.h b/Include/internal/pycore_atexit.h index 8b569422bc44bb..b4663b396852f3 100644 --- a/Include/internal/pycore_atexit.h +++ b/Include/internal/pycore_atexit.h @@ -24,8 +24,6 @@ struct _atexit_runtime_state { //################### // interpreter atexit -typedef void (*atexit_datacallbackfunc)(void *); - struct atexit_callback; typedef struct atexit_callback { atexit_datacallbackfunc func; @@ -51,9 +49,6 @@ struct atexit_state { int callback_len; }; -PyAPI_FUNC(int) _Py_AtExit( - PyInterpreterState *, atexit_datacallbackfunc, void *); - #ifdef __cplusplus } diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 3d9a2aeeb7cfd5..557a6d46ed4632 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3381,6 +3381,37 @@ test_gc_visit_objects_exit_early(PyObject *Py_UNUSED(self), } +struct atexit_data { + int called; +}; + +static void +callback(void *data) +{ + ((struct atexit_data *)data)->called += 1; +} + +static PyObject * +test_atexit(PyObject *self, PyObject *Py_UNUSED(args)) +{ + PyThreadState *oldts = PyThreadState_Swap(NULL); + PyThreadState *tstate = Py_NewInterpreter(); + + struct atexit_data data = {0}; + int res = _Py_AtExit(tstate->interp, callback, (void *)&data); + Py_EndInterpreter(tstate); + PyThreadState_Swap(oldts); + if (res < 0) { + return NULL; + } + if (data.called == 0) { + PyErr_SetString(PyExc_RuntimeError, "atexit callback not called"); + return NULL; + } + Py_RETURN_NONE; +} + + static PyObject *test_buildvalue_issue38913(PyObject *, PyObject *); static PyMethodDef TestMethods[] = { @@ -3525,6 +3556,7 @@ static PyMethodDef TestMethods[] = { {"function_set_kw_defaults", function_set_kw_defaults, METH_VARARGS, NULL}, {"test_gc_visit_objects_basic", test_gc_visit_objects_basic, METH_NOARGS, NULL}, {"test_gc_visit_objects_exit_early", test_gc_visit_objects_exit_early, METH_NOARGS, NULL}, + {"test_atexit", test_atexit, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 47a41c8eedf185..632fac2de0c419 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -12,7 +12,6 @@ #define PY_SSIZE_T_CLEAN #include "Python.h" -#include "pycore_atexit.h" // _Py_AtExit() #include "pycore_atomic_funcs.h" // _Py_atomic_int_get() #include "pycore_bitutils.h" // _Py_bswap32() #include "pycore_compile.h" // _PyCompile_CodeGen, _PyCompile_OptimizeCfg @@ -686,37 +685,6 @@ clear_extension(PyObject *self, PyObject *args) } -struct atexit_data { - int called; -}; - -static void -callback(void *data) -{ - ((struct atexit_data *)data)->called += 1; -} - -static PyObject * -test_atexit(PyObject *self, PyObject *Py_UNUSED(args)) -{ - PyThreadState *oldts = PyThreadState_Swap(NULL); - PyThreadState *tstate = Py_NewInterpreter(); - - struct atexit_data data = {0}; - int res = _Py_AtExit(tstate->interp, callback, (void *)&data); - Py_EndInterpreter(tstate); - PyThreadState_Swap(oldts); - if (res < 0) { - return NULL; - } - if (data.called == 0) { - PyErr_SetString(PyExc_RuntimeError, "atexit callback not called"); - return NULL; - } - Py_RETURN_NONE; -} - - static PyMethodDef module_functions[] = { {"get_configs", get_configs, METH_NOARGS}, {"get_recursion_depth", get_recursion_depth, METH_NOARGS}, @@ -739,7 +707,6 @@ static PyMethodDef module_functions[] = { _TESTINTERNALCAPI_OPTIMIZE_CFG_METHODDEF {"get_interp_settings", get_interp_settings, METH_VARARGS, NULL}, {"clear_extension", clear_extension, METH_VARARGS, NULL}, - {"test_atexit", test_atexit, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; From b5396e421d1bcc6f92bfc36d4aa61a9b33f18c61 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 16:22:01 -0600 Subject: [PATCH 45/49] Test a constraint. --- Modules/atexitmodule.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index 5f78eeb87acdcd..47afd7f0751039 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -27,6 +27,7 @@ int _Py_AtExit(PyInterpreterState *interp, atexit_datacallbackfunc func, void *data) { + assert(interp == _PyInterpreterState_GET()); atexit_callback *callback = PyMem_Malloc(sizeof(atexit_callback)); if (callback == NULL) { PyErr_NoMemory(); From 448b48a9c1f44cf0b261bf66bacce9cef2c5b9c0 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 17:34:03 -0600 Subject: [PATCH 46/49] Add an atexit callback for _xxinterpchannels. --- Modules/_xxinterpchannelsmodule.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Modules/_xxinterpchannelsmodule.c b/Modules/_xxinterpchannelsmodule.c index ef1cdcab952271..2f7d8e9b4b491a 100644 --- a/Modules/_xxinterpchannelsmodule.c +++ b/Modules/_xxinterpchannelsmodule.c @@ -1932,6 +1932,12 @@ _global_channels(void) { } +static void +clear_interpreter(void *data) +{ +} + + static PyObject * channel_create(PyObject *self, PyObject *Py_UNUSED(ignored)) { @@ -2339,6 +2345,10 @@ module_exec(PyObject *mod) goto error; } + // Make sure chnnels drop objects owned by this interpreter + PyInterpreterState *interp = _get_current_interp(); + _Py_AtExit(interp, clear_interpreter, (void *)interp); + return 0; error: From c86f7380047394a72a19edd8249d2860041df0f3 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 17:52:24 -0600 Subject: [PATCH 47/49] Implement the callback. --- Lib/test/test__xxinterpchannels.py | 40 ++++++++++++----- Modules/_xxinterpchannelsmodule.c | 72 ++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 11 deletions(-) diff --git a/Lib/test/test__xxinterpchannels.py b/Lib/test/test__xxinterpchannels.py index 69bda89a1688f5..b65281106f667c 100644 --- a/Lib/test/test__xxinterpchannels.py +++ b/Lib/test/test__xxinterpchannels.py @@ -550,6 +550,7 @@ def test_channel_list_interpreters_closed_send_end(self): import _xxinterpchannels as _channels _channels.close({cid}, force=True) """)) + return # Both ends should raise an error. with self.assertRaises(channels.ChannelClosedError): channels.list_interpreters(cid, send=True) @@ -673,17 +674,34 @@ def test_recv_default(self): self.assertIs(obj6, default) def test_recv_sending_interp_destroyed(self): - cid = channels.create() - interp = interpreters.create() - interpreters.run_string(interp, dedent(f""" - import _xxinterpchannels as _channels - _channels.send({cid}, b'spam') - """)) - interpreters.destroy(interp) - - with self.assertRaisesRegex(RuntimeError, - 'unrecognized interpreter ID'): - channels.recv(cid) + with self.subTest('closed'): + cid1 = channels.create() + interp = interpreters.create() + interpreters.run_string(interp, dedent(f""" + import _xxinterpchannels as _channels + _channels.send({cid1}, b'spam') + """)) + interpreters.destroy(interp) + + with self.assertRaisesRegex(RuntimeError, + f'channel {cid1} is closed'): + channels.recv(cid1) + del cid1 + with self.subTest('still open'): + cid2 = channels.create() + interp = interpreters.create() + interpreters.run_string(interp, dedent(f""" + import _xxinterpchannels as _channels + _channels.send({cid2}, b'spam') + """)) + channels.send(cid2, b'eggs') + interpreters.destroy(interp) + + channels.recv(cid2) + with self.assertRaisesRegex(RuntimeError, + f'channel {cid2} is empty'): + channels.recv(cid2) + del cid2 def test_allowed_types(self): cid = channels.create() diff --git a/Modules/_xxinterpchannelsmodule.c b/Modules/_xxinterpchannelsmodule.c index 2f7d8e9b4b491a..4756402949d249 100644 --- a/Modules/_xxinterpchannelsmodule.c +++ b/Modules/_xxinterpchannelsmodule.c @@ -489,6 +489,30 @@ _channelqueue_get(_channelqueue *queue) return _channelitem_popped(item); } +static void +_channelqueue_drop_interpreter(_channelqueue *queue, int64_t interp) +{ + _channelitem *prev = NULL; + _channelitem *next = queue->first; + while (next != NULL) { + _channelitem *item = next; + next = item->next; + if (item->data->interp == interp) { + if (prev == NULL) { + queue->first = item->next; + } + else { + prev->next = item->next; + } + _channelitem_free(item); + queue->count -= 1; + } + else { + prev = item; + } + } +} + /* channel-interpreter associations */ struct _channelend; @@ -693,6 +717,20 @@ _channelends_close_interpreter(_channelends *ends, int64_t interp, int which) return 0; } +static void +_channelends_drop_interpreter(_channelends *ends, int64_t interp) +{ + _channelend *end; + end = _channelend_find(ends->send, interp, NULL); + if (end != NULL) { + _channelends_close_end(ends, end, 1); + } + end = _channelend_find(ends->recv, interp, NULL); + if (end != NULL) { + _channelends_close_end(ends, end, 0); + } +} + static void _channelends_close_all(_channelends *ends, int which, int force) { @@ -841,6 +879,18 @@ _channel_close_interpreter(_PyChannelState *chan, int64_t interp, int end) return res; } +static void +_channel_drop_interpreter(_PyChannelState *chan, int64_t interp) +{ + PyThread_acquire_lock(chan->mutex, WAIT_LOCK); + + _channelqueue_drop_interpreter(chan->queue, interp); + _channelends_drop_interpreter(chan->ends, interp); + chan->open = _channelends_is_open(chan->ends); + + PyThread_release_lock(chan->mutex); +} + static int _channel_close_all(_PyChannelState *chan, int end, int force) { @@ -1213,6 +1263,21 @@ _channels_list_all(_channels *channels, int64_t *count) return cids; } +static void +_channels_drop_interpreter(_channels *channels, int64_t interp) +{ + PyThread_acquire_lock(channels->mutex, WAIT_LOCK); + + _channelref *ref = channels->head; + for (; ref != NULL; ref = ref->next) { + if (ref->chan != NULL) { + _channel_drop_interpreter(ref->chan, interp); + } + } + + PyThread_release_lock(channels->mutex); +} + /* support for closing non-empty channels */ struct _channel_closing { @@ -1935,6 +2000,13 @@ _global_channels(void) { static void clear_interpreter(void *data) { + if (_globals.module_count == 0) { + return; + } + PyInterpreterState *interp = (PyInterpreterState *)data; + assert(interp == _get_current_interp()); + int64_t id = PyInterpreterState_GetID(interp); + _channels_drop_interpreter(&_globals.channels, id); } From 1827feb2e7d866b1ef71e1398b1e07425dd12aba Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 17:56:33 -0600 Subject: [PATCH 48/49] Drop the _PyCrossInterpreterData_Clear() call in _xxinterpchannels. --- Modules/_xxinterpchannelsmodule.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/Modules/_xxinterpchannelsmodule.c b/Modules/_xxinterpchannelsmodule.c index 4756402949d249..13b005eaef9866 100644 --- a/Modules/_xxinterpchannelsmodule.c +++ b/Modules/_xxinterpchannelsmodule.c @@ -174,19 +174,7 @@ _release_xid_data(_PyCrossInterpreterData *data, int ignoreexc) } int res = _PyCrossInterpreterData_Release(data); if (res < 0) { - // XXX Fix this! - /* The owning interpreter is already destroyed. - * Ideally, this shouldn't ever happen. When an interpreter is - * about to be destroyed, we should clear out all of its objects - * from every channel associated with that interpreter. - * For now we hack around that to resolve refleaks, by decref'ing - * the released object here, even if its the wrong interpreter. - * The owning interpreter has already been destroyed - * so we should be okay, especially since the currently - * shareable types are all very basic, with no GC. - * That said, it becomes much messier once interpreters - * no longer share a GIL, so this needs to be fixed before then. */ - _PyCrossInterpreterData_Clear(NULL, data); + /* The owning interpreter is already destroyed. */ if (ignoreexc) { // XXX Emit a warning? PyErr_Clear(); From 82b395cc4dc3ad498f1fcc35eebacd02024d5e47 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 17:59:39 -0600 Subject: [PATCH 49/49] Drop the _PyCrossInterpreterData_Clear() call in _xxsubinterpreters. --- Modules/_xxsubinterpretersmodule.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index 11164676c4d107..884fb0d31f2b7f 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -67,16 +67,7 @@ _release_xid_data(_PyCrossInterpreterData *data, int ignoreexc) } int res = _PyCrossInterpreterData_Release(data); if (res < 0) { - // XXX Fix this! - /* The owning interpreter is already destroyed. - * Ideally, this shouldn't ever happen. (It's highly unlikely.) - * For now we hack around that to resolve refleaks, by decref'ing - * the released object here, even if its the wrong interpreter. - * The owning interpreter has already been destroyed - * so we should be okay, especially since the currently - * shareable types are all very basic, with no GC. - * That said, it becomes much messier once interpreters - * no longer share a GIL, so this needs to be fixed before then. */ + /* The owning interpreter is already destroyed. */ _PyCrossInterpreterData_Clear(NULL, data); if (ignoreexc) { // XXX Emit a warning?