Skip to content

gh-101659: Add _Py_AtExit() #103298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Apr 6, 2023
4 changes: 4 additions & 0 deletions Include/cpython/pylifecycle.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 *);
56 changes: 56 additions & 0 deletions Include/internal/pycore_atexit.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
#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


//###############
// runtime atexit

typedef void (*atexit_callbackfunc)(void);

struct _atexit_runtime_state {
#define NEXITFUNCS 32
atexit_callbackfunc callbacks[NEXITFUNCS];
int ncallbacks;
};


//###################
// interpreter atexit

struct atexit_callback;
typedef struct atexit_callback {
atexit_datacallbackfunc func;
void *data;
struct atexit_callback *next;
} atexit_callback;

typedef struct {
PyObject *func;
PyObject *args;
PyObject *kwargs;
} 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;
};


#ifdef __cplusplus
}
#endif
#endif /* !Py_INTERNAL_ATEXIT_H */
17 changes: 2 additions & 15 deletions Include/internal/pycore_interp.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ extern "C" {

#include <stdbool.h>

#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
Expand All @@ -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_callback;

struct atexit_state {
atexit_callback **callbacks;
int ncallbacks;
int callback_len;
};


struct _Py_long_state {
int max_str_digits;
};
Expand Down
5 changes: 2 additions & 3 deletions Include/internal/pycore_runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
40 changes: 29 additions & 11 deletions Lib/test/test__xxinterpchannels.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions Makefile.pre.in
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
32 changes: 32 additions & 0 deletions Modules/_testcapimodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = {
Expand Down Expand Up @@ -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 */
};

Expand Down
96 changes: 83 additions & 13 deletions Modules/_xxinterpchannelsmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -489,6 +477,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;
Expand Down Expand Up @@ -693,6 +705,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)
{
Expand Down Expand Up @@ -841,6 +867,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)
{
Expand Down Expand Up @@ -1213,6 +1251,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 {
Expand Down Expand Up @@ -1932,6 +1985,19 @@ _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);
}


static PyObject *
channel_create(PyObject *self, PyObject *Py_UNUSED(ignored))
{
Expand Down Expand Up @@ -2339,6 +2405,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:
Expand Down
11 changes: 1 addition & 10 deletions Modules/_xxsubinterpretersmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
Loading