From b01459b1284b25617eec05fc1a389eff7ec22af1 Mon Sep 17 00:00:00 2001
From: Jean Boussier <jean.boussier@gmail.com>
Date: Tue, 25 Jan 2022 13:19:08 +0100
Subject: [PATCH] Basic unregister GVL callbacks API (thread unsafe)

---
 .../gvl/call_without_gvl/call_without_gvl.c   | 31 ++++++++++++++++++-
 include/ruby/thread_native.h                  | 16 +++++++++-
 test/-ext-/gvl/test_last_thread.rb            | 11 ++++++-
 thread_pthread.c                              | 24 +++++++++++++-
 thread_pthread.h                              | 11 -------
 5 files changed, 78 insertions(+), 15 deletions(-)

diff --git a/ext/-test-/gvl/call_without_gvl/call_without_gvl.c b/ext/-test-/gvl/call_without_gvl/call_without_gvl.c
index e845914782d8ae..0fc35ca29416d6 100644
--- a/ext/-test-/gvl/call_without_gvl/call_without_gvl.c
+++ b/ext/-test-/gvl/call_without_gvl/call_without_gvl.c
@@ -74,10 +74,21 @@ ex_callback(uint32_t e, struct gvl_hook_event_args args) {
     fprintf(stderr, "calling callback\n");
 }
 
+static gvl_hook_t * single_hook = NULL;
+
 static VALUE
 thread_register_gvl_callback(VALUE thread) {
-    rb_gvl_event_new(*ex_callback, 0x12);
+    single_hook = rb_gvl_event_new(*ex_callback, 0x12);
+
+    return Qnil;
+}
 
+static VALUE
+thread_unregister_gvl_callback(VALUE thread) {
+    if (single_hook) {
+        rb_gvl_event_delete(single_hook);
+        single_hook = NULL;
+    }
 
     return Qnil;
 }
@@ -88,6 +99,21 @@ thread_call_gvl_callback(VALUE thread) {
     return Qnil;
 }
 
+static VALUE
+thread_register_and_unregister_gvl_callback(VALUE thread) {
+    gvl_hook_t * hooks[5];
+    for (int i = 0; i < 5; i++) {
+        hooks[i] = rb_gvl_event_new(*ex_callback, 0x12);
+    }
+
+    if (!rb_gvl_event_delete(hooks[4])) return Qfalse;
+    if (!rb_gvl_event_delete(hooks[0])) return Qfalse;
+    if (!rb_gvl_event_delete(hooks[3])) return Qfalse;
+    if (!rb_gvl_event_delete(hooks[2])) return Qfalse;
+    if (!rb_gvl_event_delete(hooks[1])) return Qfalse;
+    return Qtrue;
+}
+
 void
 Init_call_without_gvl(void)
 {
@@ -96,5 +122,8 @@ Init_call_without_gvl(void)
     rb_define_singleton_method(klass, "runnable_sleep", thread_runnable_sleep, 1);
     rb_define_singleton_method(klass, "ubf_async_safe", thread_ubf_async_safe, 1);
     rb_define_singleton_method(klass, "register_callback", thread_register_gvl_callback, 0);
+    rb_define_singleton_method(klass, "unregister_callback", thread_unregister_gvl_callback, 0);
+    rb_define_singleton_method(klass, "register_and_unregister_callbacks", thread_register_and_unregister_gvl_callback, 0);
     rb_define_singleton_method(klass, "call_callbacks", thread_call_gvl_callback, 0);
+    
 }
diff --git a/include/ruby/thread_native.h b/include/ruby/thread_native.h
index ae1df02343f7ee..d722013ea0f470 100644
--- a/include/ruby/thread_native.h
+++ b/include/ruby/thread_native.h
@@ -205,8 +205,22 @@ void rb_native_cond_destroy(rb_nativethread_cond_t *cond);
 struct gvl_hook_event_args {
     //
 };
+#include <stdint.h>
+
 typedef void (*rb_gvl_callback)(uint32_t event, struct gvl_hook_event_args args);
-void rb_gvl_event_new(void *callback, uint32_t event);
+
+// TODO: this is going to be the same on Windows so move it somewhere sensible
+typedef struct gvl_hook {
+    rb_gvl_callback callback;
+    uint32_t event;
+
+    struct gvl_hook *next;
+} gvl_hook_t;
+
+#include "ruby/internal/memory.h"
+
+gvl_hook_t * rb_gvl_event_new(void *callback, uint32_t event);
+bool rb_gvl_event_delete(gvl_hook_t * hook);
 void rb_gvl_execute_hooks(uint32_t event);
 RBIMPL_SYMBOL_EXPORT_END()
 #endif
diff --git a/test/-ext-/gvl/test_last_thread.rb b/test/-ext-/gvl/test_last_thread.rb
index 791e13834d581a..dab8783e568992 100644
--- a/test/-ext-/gvl/test_last_thread.rb
+++ b/test/-ext-/gvl/test_last_thread.rb
@@ -23,7 +23,16 @@ def test_gvl_instrumentation
     require '-test-/gvl/call_without_gvl'
     Bug::Thread::register_callback
 
-    Bug::Thread::call_callbacks
+    begin
+      Bug::Thread::call_callbacks
+    ensure
+      Bug::Thread::unregister_callback
+    end
+  end
+
+  def test_gvl_instrumentation_unregister
+    require '-test-/gvl/call_without_gvl'
+    assert Bug::Thread::register_and_unregister_callbacks
   end
 end
 
diff --git a/thread_pthread.c b/thread_pthread.c
index 5665afbe727679..8672ab36237b9a 100644
--- a/thread_pthread.c
+++ b/thread_pthread.c
@@ -103,7 +103,7 @@
 
 static gvl_hook_t * rb_gvl_hooks = NULL;
 
-void
+gvl_hook_t *
 rb_gvl_event_new(void *callback, uint32_t event) {
     gvl_hook_t *hook = ALLOC_N(gvl_hook_t, 1);
     hook->callback = callback;
@@ -115,6 +115,28 @@ rb_gvl_event_new(void *callback, uint32_t event) {
         hook->next = rb_gvl_hooks;
         rb_gvl_hooks = hook;
     }
+    return hook;
+}
+
+bool
+rb_gvl_event_delete(gvl_hook_t * hook) {
+    if (rb_gvl_hooks == hook) {
+        rb_gvl_hooks = hook->next;
+        ruby_xfree(hook);
+        return TRUE;
+    }
+    
+    gvl_hook_t *h = rb_gvl_hooks;
+
+    do {
+        if (h->next == hook) {
+            h->next = hook->next;
+            ruby_xfree(hook);
+            return TRUE;
+        }
+    } while ((h = h->next));
+
+    return FALSE;
 }
 
 void
diff --git a/thread_pthread.h b/thread_pthread.h
index 240c8d9fe453e6..2ac354046c010b 100644
--- a/thread_pthread.h
+++ b/thread_pthread.h
@@ -69,17 +69,6 @@ typedef struct rb_global_vm_lock_struct {
     int wait_yield;
 } rb_global_vm_lock_t;
 
-#include <stdint.h>
-
-// TODO: this is going to be the same on Windows so move it somewhere sensible
-typedef struct gvl_hook {
-    rb_gvl_callback callback;
-    uint32_t event;
-
-    struct gvl_hook *next;
-} gvl_hook_t;
-
-#include "ruby/internal/memory.h"
 
 #if __STDC_VERSION__ >= 201112
   #define RB_THREAD_LOCAL_SPECIFIER _Thread_local