Skip to content

Commit f47d87e

Browse files
committed
Guard against writing to reclaimed graalpy_finalizing memory
1 parent 5b070a7 commit f47d87e

File tree

3 files changed

+21
-8
lines changed
  • graalpython

3 files changed

+21
-8
lines changed

graalpython/com.oracle.graal.python.cext/src/capi.c

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -951,7 +951,7 @@ Py_LOCAL_SYMBOL TruffleContext* TRUFFLE_CONTEXT;
951951
* that do not work during context shutdown! (This means that it would be safe
952952
* to share this global across multiple contexts.)
953953
*/
954-
Py_LOCAL_SYMBOL int32_t graalpy_finalizing;
954+
Py_LOCAL_SYMBOL int8_t *_graalpy_finalizing = NULL;
955955

956956
PyAPI_FUNC(void) initialize_graal_capi(TruffleEnv* env, void **builtin_closures, GCState *gc) {
957957
clock_t t = clock();
@@ -1007,10 +1007,16 @@ PyAPI_FUNC(void) initialize_graal_capi(TruffleEnv* env, void **builtin_closures,
10071007

10081008
/*
10091009
* This function is called from Java during C API initialization to get the
1010-
* pointer `graalpy_finalizing`.
1010+
* pointer `_graalpy_finalizing`.
10111011
*/
1012-
PyAPI_FUNC(int32_t *) GraalPy_get_finalize_capi_pointer() {
1013-
return &graalpy_finalizing;
1012+
PyAPI_FUNC(int8_t *) GraalPy_get_finalize_capi_pointer() {
1013+
assert(!_graalpy_finalizing);
1014+
// We actually leak this memory on purpose. On the Java side, this is
1015+
// written to in a VM shutdown hook. Once such a hook is registered it
1016+
// sticks around, so we're leaking those hooks anyway. 1 byte more to leak
1017+
// does not strike me (timfel) as significant.
1018+
_graalpy_finalizing = (int8_t *)calloc(1, sizeof(int8_t));
1019+
return _graalpy_finalizing;
10141020
}
10151021

10161022
#if ((__linux__ && __GNU_LIBRARY__) || __APPLE__)

graalpython/com.oracle.graal.python.cext/src/capi.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -157,7 +157,8 @@ PyAPI_DATA(uint32_t) Py_Truffle_Options;
157157

158158
extern THREAD_LOCAL Py_LOCAL_SYMBOL PyThreadState *tstate_current;
159159

160-
extern Py_LOCAL_SYMBOL int graalpy_finalizing;
160+
extern Py_LOCAL_SYMBOL int8_t *_graalpy_finalizing;
161+
#define graalpy_finalizing (_graalpy_finalizing != NULL && *_graalpy_finalizing)
161162

162163
/* Flags definitions representing global (debug) options. */
163164
static MUST_INLINE int PyTruffle_Trace_Memory() {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CApiContext.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1134,6 +1134,11 @@ public static Object loadCExtModule(Node location, PythonContext context, Module
11341134
* know that it's not safe to do upcalls and that native wrappers might have been deallocated.
11351135
* We need to do it in a VM shutdown hook to make sure C atexit won't crash even if our context
11361136
* finalization didn't run.
1137+
*
1138+
* The memory of the shared library may have been re-used if the GraalPy context was shut down
1139+
* (cleanly or not), the sources were collected, and NFI's mechanism for unloading libraries
1140+
* triggered a dlclose that dropped the refcount of the python-native library to 0. We leak 1
1141+
* byte of memory and this shutdown hook for each context that ever initialized the C API.
11371142
*/
11381143
private void addNativeFinalizer(PythonContext context, Object finalizingPointerObj) {
11391144
final Unsafe unsafe = context.getUnsafe();
@@ -1143,7 +1148,8 @@ private void addNativeFinalizer(PythonContext context, Object finalizingPointerO
11431148
long finalizingPointer = lib.asPointer(finalizingPointerObj);
11441149
// We are writing off heap memory and registering a VM shutdown hook, there is no
11451150
// point in creating this thread via Truffle sandbox at this point
1146-
nativeFinalizerRunnable = () -> unsafe.putInt(finalizingPointer, 1);
1151+
nativeFinalizerRunnable = () -> unsafe.putByte(finalizingPointer, (byte) 1);
1152+
context.registerAtexitHook((c) -> nativeFinalizerRunnable.run());
11471153
nativeFinalizerShutdownHook = new Thread(nativeFinalizerRunnable);
11481154
Runtime.getRuntime().addShutdownHook(nativeFinalizerShutdownHook);
11491155
} catch (UnsupportedMessageException e) {

0 commit comments

Comments
 (0)