Skip to content

Commit

Permalink
gh-101659: Isolate "obmalloc" State to Each Interpreter (gh-101660)
Browse files Browse the repository at this point in the history
This is strictly about moving the "obmalloc" runtime state from
`_PyRuntimeState` to `PyInterpreterState`.  Doing so improves isolation
between interpreters, specifically most of the memory (incl. objects)
allocated for each interpreter's use.  This is important for a
per-interpreter GIL, but such isolation is valuable even without it.

FWIW, a per-interpreter obmalloc is the proverbial
canary-in-the-coalmine when it comes to the isolation of objects between
interpreters.  Any object that leaks (unintentionally) to another
interpreter is highly likely to cause a crash (on debug builds at
least).  That's a useful thing to know, relative to interpreter
isolation.
  • Loading branch information
ericsnowcurrently authored Apr 24, 2023
1 parent 01be52e commit df3173d
Show file tree
Hide file tree
Showing 20 changed files with 322 additions and 73 deletions.
4 changes: 4 additions & 0 deletions Include/cpython/initconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,8 @@ 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;
int allow_threads;
Expand All @@ -254,6 +256,7 @@ typedef struct {

#define _PyInterpreterConfig_INIT \
{ \
.use_main_obmalloc = 0, \
.allow_fork = 0, \
.allow_exec = 0, \
.allow_threads = 1, \
Expand All @@ -263,6 +266,7 @@ typedef struct {

#define _PyInterpreterConfig_LEGACY_INIT \
{ \
.use_main_obmalloc = 1, \
.allow_fork = 1, \
.allow_exec = 1, \
.allow_threads = 1, \
Expand Down
4 changes: 4 additions & 0 deletions Include/cpython/pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
5 changes: 4 additions & 1 deletion Include/internal/pycore_interp.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@ 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_global_objects.h" // struct _Py_interp_static_objects
#include "pycore_import.h" // struct _import_state
#include "pycore_instruments.h" // PY_MONITORING_EVENTS
#include "pycore_list.h" // struct _Py_list_state
#include "pycore_global_objects.h" // struct _Py_interp_static_objects
#include "pycore_object_state.h" // struct _py_object_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
Expand Down Expand Up @@ -82,6 +83,8 @@ struct _is {
int _initialized;
int finalizing;

struct _obmalloc_state obmalloc;

struct _ceval_state ceval;
struct _gc_runtime_state gc;

Expand Down
12 changes: 10 additions & 2 deletions Include/internal/pycore_obmalloc.h
Original file line number Diff line number Diff line change
Expand Up @@ -657,8 +657,12 @@ struct _obmalloc_usage {
#endif /* WITH_PYMALLOC_RADIX_TREE */


struct _obmalloc_state {
struct _obmalloc_global_state {
int dump_debug_stats;
Py_ssize_t interpreter_leaks;
};

struct _obmalloc_state {
struct _obmalloc_pools pools;
struct _obmalloc_mgmt mgmt;
struct _obmalloc_usage usage;
Expand All @@ -675,7 +679,11 @@ 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 *);
extern void _PyInterpreterState_FinalizeAllocatedBlocks(PyInterpreterState *);


#ifdef WITH_PYMALLOC
Expand Down
6 changes: 5 additions & 1 deletion Include/internal/pycore_obmalloc_init.h
Original file line number Diff line number Diff line change
Expand Up @@ -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), \
}, \
Expand Down
1 change: 1 addition & 0 deletions Include/internal/pycore_pylifecycle.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 7 additions & 0 deletions Include/internal/pycore_pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions Include/internal/pycore_runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,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
Expand Down Expand Up @@ -88,7 +87,7 @@ typedef struct pyruntimestate {
_Py_atomic_address _finalizing;

struct _pymem_allocators allocators;
struct _obmalloc_state obmalloc;
struct _obmalloc_global_state obmalloc;
struct pyhash_runtime_state pyhash_state;
struct _time_runtime_state time;
struct _pythread_runtime_state threads;
Expand Down
3 changes: 2 additions & 1 deletion Include/internal/pycore_runtime_init.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ extern PyTypeObject _PyExc_MemoryError;
_pymem_allocators_debug_INIT, \
_pymem_allocators_obj_arena_INIT, \
}, \
.obmalloc = _obmalloc_state_INIT(runtime.obmalloc), \
.obmalloc = _obmalloc_global_state_INIT, \
.pyhash_state = pyhash_state_INIT, \
.signals = _signals_RUNTIME_INIT, \
.interpreters = { \
Expand Down Expand Up @@ -93,6 +93,7 @@ extern PyTypeObject _PyExc_MemoryError;
{ \
.id_refcount = -1, \
.imports = IMPORTS_INIT, \
.obmalloc = _obmalloc_state_INIT(INTERP.obmalloc), \
.ceval = { \
.recursion_limit = Py_DEFAULT_RECURSION_LIMIT, \
}, \
Expand Down
33 changes: 27 additions & 6 deletions Lib/test/test_capi/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1211,20 +1211,25 @@ 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'

# expected to work
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,
(True, False, False, False, False, False): OBMALLOC,
(False, False, False, True, False, True): THREADS | EXTENSIONS,
}.items():
kwargs = dict(zip(kwlist, config))
expected = {
Expand All @@ -1246,6 +1251,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):
Expand All @@ -1257,13 +1276,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,
Expand Down
3 changes: 2 additions & 1 deletion Lib/test/test_embed.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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',
Expand Down
27 changes: 23 additions & 4 deletions Lib/test/test_import/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1636,7 +1636,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):
Expand Down Expand Up @@ -1669,6 +1674,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.
Expand All @@ -1689,6 +1695,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,
)

Expand All @@ -1699,33 +1706,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'''
Expand All @@ -1743,12 +1753,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'''
Expand Down Expand Up @@ -1854,6 +1865,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)


class TestSinglePhaseSnapshot(ModuleSnapshot):

Expand Down
1 change: 1 addition & 0 deletions Lib/test/test_threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
12 changes: 10 additions & 2 deletions Modules/_testcapimodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -1493,19 +1494,25 @@ 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",
"allow_daemon_threads",
"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;
Expand All @@ -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,
Expand Down
Loading

0 comments on commit df3173d

Please sign in to comment.