Debug mode: detect HPy_Close/return of context constants

hpy/debug/src/debug_handles.c

@@ -31,7 +31,7 @@ static void DebugHandle_free_raw_data(HPyDebugInfo *info, DebugHandle *handle, b
     }
 }
 
-DHPy DHPy_open(HPyContext *dctx, UHPy uh)
+static DHPy _DHPy_open(HPyContext *dctx, UHPy uh, bool is_context_contant)
 {
     UHPy_sanity_check(uh);
     if (HPy_IsNull(uh))
@@ -54,12 +54,22 @@ DHPy DHPy_open(HPyContext *dctx, UHPy uh)
     handle->uh = uh;
     handle->generation = info->current_generation;
     handle->is_closed = 0;
+    handle->is_context_constant = is_context_contant;
     handle->associated_data = NULL;
     DHQueue_append(&info->open_handles, handle);
     debug_handles_sanity_check(info);
     return as_DHPy(handle);
 }
 
+DHPy DHPy_open_context_constant(HPyContext *dctx, UHPy uh) {
+    return _DHPy_open(dctx, uh, true);
+}
+
+DHPy DHPy_open(HPyContext *dctx, UHPy uh)
+{
+    return _DHPy_open(dctx, uh, false);
+}
+
 static void print_error(HPyContext *uctx, const char *message)
 {
     // We don't have a way to propagate exceptions from within DHPy_unwrap, so
@@ -75,7 +85,7 @@ void DHPy_invalid_handle(HPyContext *dctx, DHPy dh)
 {
     HPyDebugInfo *info = get_info(dctx);
     HPyContext *uctx = info->uctx;
-    assert(as_DebugHandle(dh)->is_closed);
+    assert(as_DebugHandle(dh)->is_closed || as_DebugHandle(dh)->is_context_constant);
     if (HPy_IsNull(info->uh_on_invalid_handle)) {
         // default behavior: print an error and abort
         HPy_FatalError(uctx, "Invalid usage of already closed handle");
@@ -129,6 +139,10 @@ void DHPy_close(HPyContext *dctx, DHPy dh)
     if (handle->is_closed)
         return;
 
+    // Closing context constants is never allowed
+    if (handle->is_context_constant)
+        DHPy_invalid_handle(dctx, dh);
+
     // move the handle from open_handles to closed_handles
     DHQueue_remove(&info->open_handles, handle);
     DHQueue_append(&info->closed_handles, handle);

hpy/debug/src/debug_internal.h

@@ -137,7 +137,8 @@ static inline void UHPy_sanity_check(UHPy uh) {
 typedef struct DebugHandle {
     UHPy uh;
     long generation;
-    bool is_closed;
+    bool is_closed:1;
+    bool is_context_constant:1;
     // pointer to and size of any raw data associated with
     // the lifetime of the handle:
     void *associated_data;
@@ -156,6 +157,7 @@ static inline DHPy as_DHPy(DebugHandle *handle) {
 }
 
 DHPy DHPy_open(HPyContext *dctx, UHPy uh);
+DHPy DHPy_open_context_constant(HPyContext *dctx, UHPy uh);
 void DHPy_close(HPyContext *dctx, DHPy dh);
 void DHPy_close_and_check(HPyContext *dctx, DHPy dh);
 void DHPy_free(HPyContext *dctx, DHPy dh);
@@ -195,13 +197,15 @@ typedef struct {
     HPyContext *uctx;
     long current_generation;
 
-    // the following should be an HPyField, but it's complicate:
+    // the following should be an HPyField, but it's complicated:
     // HPyFields should be used only on memory which is known by the GC, which
     // happens automatically if you use e.g. HPy_New, but currently
     // HPy_DebugInfo is malloced(). We need either:
     //   1. a generic HPy_GcMalloc() OR
     //   2. HPy_{Un}TrackMemory(), so that we can add manually allocated
     //      memory as a GC root
+    // Alternative is to put it into a module state or to put it into HPyGlobal
+    // once those features are implemented
     UHPy uh_on_invalid_handle;
     HPy_ssize_t closed_handles_queue_max_size; // configurable by the user
     HPy_ssize_t protected_raw_data_max_size;

hpy/tools/autogen/debug.py

@@ -47,7 +47,7 @@ def generate(self):
         w('{')
         for var in self.api.variables:
             name = var.name
-            w(f'    dctx->{name} = DHPy_open(dctx, uctx->{name});')
+            w(f'    dctx->{name} = DHPy_open_context_constant(dctx, uctx->{name});')
         for func in self.api.functions:
             name = func.ctx_name()
             w(f'    dctx->{name} = &debug_{name};')

test/debug/test_handles_invalid.py

@@ -135,6 +135,49 @@ def test_keeping_and_reusing_argument_handle(compiler, hpy_debug_capture):
     assert hpy_debug_capture.invalid_handles_count == 1
 
 
+def test_return_ctx_constant_without_dup(compiler, python_subprocess, fatal_exit_code):
+    # Since this puts the context->h_None into an inconsistent state, we run
+    # this test in a subprocess and check fatal error instead
+    if not SUPPORTS_SYS_EXECUTABLE:
+        pytest.skip("no sys.executable")
+
+    mod = compiler.compile_module("""
+        HPyDef_METH(f, "f", f_impl, HPyFunc_NOARGS)
+        static HPy f_impl(HPyContext *ctx, HPy self)
+        {
+            return ctx->h_None;
+        }
+
+        @EXPORT(f)
+        @INIT
+    """)
+    result = python_subprocess.run(mod, "mod.f();")
+    assert result.returncode == fatal_exit_code
+    assert b"Invalid usage of already closed handle" in result.stderr
+
+
+def test_close_ctx_constant(compiler, python_subprocess, fatal_exit_code):
+    # Since this puts the context->h_True into an inconsistent state, we run
+    # this test in a subprocess and check fatal error instead
+    if not SUPPORTS_SYS_EXECUTABLE:
+        pytest.skip("no sys.executable")
+
+    mod = compiler.compile_module("""
+        HPyDef_METH(f, "f", f_impl, HPyFunc_NOARGS)
+        static HPy f_impl(HPyContext *ctx, HPy self)
+        {
+            HPy_Close(ctx, ctx->h_True);
+            return HPy_Dup(ctx, ctx->h_False);
+        }
+
+        @EXPORT(f)
+        @INIT
+    """)
+    result = python_subprocess.run(mod, "mod.f();")
+    assert result.returncode == fatal_exit_code
+    assert b"Invalid usage of already closed handle" in result.stderr
+
+
 def test_invalid_handle_crashes_python_if_no_hook(compiler, python_subprocess, fatal_exit_code):
     if not SUPPORTS_SYS_EXECUTABLE:
         pytest.skip("no sys.executable")
@@ -153,4 +196,5 @@ def test_invalid_handle_crashes_python_if_no_hook(compiler, python_subprocess, f
         @INIT
     """)
     result = python_subprocess.run(mod, "mod.f(42);")
-    assert result.returncode == fatal_exit_code
+    assert result.returncode == fatal_exit_code
+    assert b"Invalid usage of already closed handle" in result.stderr