https://git.ruby-lang.org/ruby.git/commit/?id=2e6e2fd9da

From 2e6e2fd9da18b74aa9555d09a871b24895e42773 Mon Sep 17 00:00:00 2001
From: Koichi Sasada <ko1@a...>
Date: Mon, 13 Dec 2021 02:15:05 +0900
Subject: fix local TP memory leak

It free `rb_hook_list_t` itself if needed. To recognize the
need, this patch introduced `rb_hook_list_t::is_local` flag.

This patch is succession of https://github.com/ruby/ruby/pull/4652
---
 iseq.c     |  5 ++---
 vm_core.h  |  3 ++-
 vm_trace.c | 37 +++++++++++++++++++++++--------------
 3 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/iseq.c b/iseq.c
index 21622fa288b..9538847577a 100644
--- a/iseq.c
+++ b/iseq.c
@@ -3359,6 +3359,7 @@ iseq_add_local_tracepoint(const rb_iseq_t *iseq, rb_event_flag_t turnon_events, https://github.com/ruby/ruby/blob/trunk/iseq.c#L3359
     if (n > 0) {
         if (iseq->aux.exec.local_hooks == NULL) {
             ((rb_iseq_t *)iseq)->aux.exec.local_hooks = RB_ZALLOC(rb_hook_list_t);
+            iseq->aux.exec.local_hooks->is_local = true;
         }
         rb_hook_list_connect_tracepoint((VALUE)iseq, iseq->aux.exec.local_hooks, tpval, target_line);
     }
@@ -3413,9 +3414,7 @@ iseq_remove_local_tracepoint(const rb_iseq_t *iseq, VALUE tpval) https://github.com/ruby/ruby/blob/trunk/iseq.c#L3414
         local_events = iseq->aux.exec.local_hooks->events;
 
         if (local_events == 0) {
-            if (iseq->aux.exec.local_hooks->running == 0) {
-                rb_hook_list_free(iseq->aux.exec.local_hooks);
-            }
+            rb_hook_list_free(iseq->aux.exec.local_hooks);
             ((rb_iseq_t *)iseq)->aux.exec.local_hooks = NULL;
         }
 
diff --git a/vm_core.h b/vm_core.h
index 78d06012a14..6bbce23078f 100644
--- a/vm_core.h
+++ b/vm_core.h
@@ -609,8 +609,9 @@ void rb_objspace_call_finalizer(struct rb_objspace *); https://github.com/ruby/ruby/blob/trunk/vm_core.h#L609
 typedef struct rb_hook_list_struct {
     struct rb_event_hook_struct *hooks;
     rb_event_flag_t events;
-    unsigned int need_clean;
     unsigned int running;
+    bool need_clean;
+    bool is_local;
 } rb_hook_list_t;
 
 
diff --git a/vm_trace.c b/vm_trace.c
index 3f589d3778d..466856341d0 100644
--- a/vm_trace.c
+++ b/vm_trace.c
@@ -69,8 +69,11 @@ static void clean_hooks(const rb_execution_context_t *ec, rb_hook_list_t *list); https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L69
 void
 rb_hook_list_free(rb_hook_list_t *hooks)
 {
-    hooks->need_clean = TRUE;
-    clean_hooks(GET_EC(), hooks);
+    hooks->need_clean = true;
+
+    if (hooks->running == 0) {
+        clean_hooks(GET_EC(), hooks);
+    }
 }
 
 /* ruby_vm_event_flags management */
@@ -200,10 +203,13 @@ static void https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L203
 clean_hooks(const rb_execution_context_t *ec, rb_hook_list_t *list)
 {
     rb_event_hook_t *hook, **nextp = &list->hooks;
-    VM_ASSERT(list->need_clean == TRUE);
     rb_event_flag_t prev_events = list->events;
+
+    VM_ASSERT(list->running == 0);
+    VM_ASSERT(list->need_clean == true);
+
     list->events = 0;
-    list->need_clean = FALSE;
+    list->need_clean = false;
 
     while ((hook = *nextp) != 0) {
 	if (hook->hook_flags & RUBY_EVENT_HOOK_FLAG_DELETED) {
@@ -216,19 +222,21 @@ clean_hooks(const rb_execution_context_t *ec, rb_hook_list_t *list) https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L222
 	}
     }
 
-    if (list == rb_ec_ractor_hooks(ec)) {
-        /* global events */
-        update_global_event_hook(prev_events, list->events);
+    if (list->is_local) {
+        if (list->events == 0) {
+            /* local events */
+            ruby_xfree(list);
+        }
     }
     else {
-        /* local events */
+        update_global_event_hook(prev_events, list->events);
     }
 }
 
 static void
 clean_hooks_check(const rb_execution_context_t *ec, rb_hook_list_t *list)
 {
-    if (UNLIKELY(list->need_clean != FALSE)) {
+    if (UNLIKELY(list->need_clean)) {
         if (list->running == 0) {
             clean_hooks(ec, list);
         }
@@ -251,7 +259,7 @@ remove_event_hook(const rb_execution_context_t *ec, const rb_thread_t *filter_th https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L259
 		if (data == Qundef || hook->data == data) {
 		    hook->hook_flags |= RUBY_EVENT_HOOK_FLAG_DELETED;
 		    ret+=1;
-		    list->need_clean = TRUE;
+		    list->need_clean = true;
 		}
 	    }
 	}
@@ -1249,10 +1257,11 @@ disable_local_event_iseq_i(VALUE target, VALUE iseq_p, VALUE tpval) https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L1257
         rb_hook_list_t *hooks = def->body.bmethod.hooks;
         VM_ASSERT(hooks != NULL);
         rb_hook_list_remove_tracepoint(hooks, tpval);
-        if (hooks->running == 0) {
+
+        if (hooks->events == 0) {
             rb_hook_list_free(def->body.bmethod.hooks);
+            def->body.bmethod.hooks = NULL;
         }
-        def->body.bmethod.hooks = NULL;
     }
     return ST_CONTINUE;
 }
@@ -1301,9 +1310,9 @@ rb_hook_list_remove_tracepoint(rb_hook_list_t *list, VALUE tpval) https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L1310
     while (hook) {
         if (hook->data == tpval) {
             hook->hook_flags |= RUBY_EVENT_HOOK_FLAG_DELETED;
-            list->need_clean = TRUE;
+            list->need_clean = true;
         }
-        else {
+        else if ((hook->hook_flags & RUBY_EVENT_HOOK_FLAG_DELETED) == 0) {
             events |= hook->events;
         }
         hook = hook->next;
-- 
cgit v1.2.1


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/