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
This commit is contained in:
Koichi Sasada 2021-12-13 02:15:05 +09:00
parent b8f7fc361d
commit 2e6e2fd9da
Notes: git 2021-12-15 02:32:17 +09:00
3 changed files with 27 additions and 18 deletions

5
iseq.c
View File

@ -3359,6 +3359,7 @@ iseq_add_local_tracepoint(const rb_iseq_t *iseq, rb_event_flag_t turnon_events,
if (n > 0) { if (n > 0) {
if (iseq->aux.exec.local_hooks == NULL) { if (iseq->aux.exec.local_hooks == NULL) {
((rb_iseq_t *)iseq)->aux.exec.local_hooks = RB_ZALLOC(rb_hook_list_t); ((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); 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)
local_events = iseq->aux.exec.local_hooks->events; local_events = iseq->aux.exec.local_hooks->events;
if (local_events == 0) { 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; ((rb_iseq_t *)iseq)->aux.exec.local_hooks = NULL;
} }

View File

@ -609,8 +609,9 @@ void rb_objspace_call_finalizer(struct rb_objspace *);
typedef struct rb_hook_list_struct { typedef struct rb_hook_list_struct {
struct rb_event_hook_struct *hooks; struct rb_event_hook_struct *hooks;
rb_event_flag_t events; rb_event_flag_t events;
unsigned int need_clean;
unsigned int running; unsigned int running;
bool need_clean;
bool is_local;
} rb_hook_list_t; } rb_hook_list_t;

View File

@ -69,8 +69,11 @@ static void clean_hooks(const rb_execution_context_t *ec, rb_hook_list_t *list);
void void
rb_hook_list_free(rb_hook_list_t *hooks) rb_hook_list_free(rb_hook_list_t *hooks)
{ {
hooks->need_clean = TRUE; hooks->need_clean = true;
clean_hooks(GET_EC(), hooks);
if (hooks->running == 0) {
clean_hooks(GET_EC(), hooks);
}
} }
/* ruby_vm_event_flags management */ /* ruby_vm_event_flags management */
@ -200,10 +203,13 @@ static void
clean_hooks(const rb_execution_context_t *ec, rb_hook_list_t *list) clean_hooks(const rb_execution_context_t *ec, rb_hook_list_t *list)
{ {
rb_event_hook_t *hook, **nextp = &list->hooks; rb_event_hook_t *hook, **nextp = &list->hooks;
VM_ASSERT(list->need_clean == TRUE);
rb_event_flag_t prev_events = list->events; rb_event_flag_t prev_events = list->events;
VM_ASSERT(list->running == 0);
VM_ASSERT(list->need_clean == true);
list->events = 0; list->events = 0;
list->need_clean = FALSE; list->need_clean = false;
while ((hook = *nextp) != 0) { while ((hook = *nextp) != 0) {
if (hook->hook_flags & RUBY_EVENT_HOOK_FLAG_DELETED) { 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)
} }
} }
if (list == rb_ec_ractor_hooks(ec)) { if (list->is_local) {
/* global events */ if (list->events == 0) {
update_global_event_hook(prev_events, list->events); /* local events */
ruby_xfree(list);
}
} }
else { else {
/* local events */ update_global_event_hook(prev_events, list->events);
} }
} }
static void static void
clean_hooks_check(const rb_execution_context_t *ec, rb_hook_list_t *list) 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) { if (list->running == 0) {
clean_hooks(ec, list); clean_hooks(ec, list);
} }
@ -251,7 +259,7 @@ remove_event_hook(const rb_execution_context_t *ec, const rb_thread_t *filter_th
if (data == Qundef || hook->data == data) { if (data == Qundef || hook->data == data) {
hook->hook_flags |= RUBY_EVENT_HOOK_FLAG_DELETED; hook->hook_flags |= RUBY_EVENT_HOOK_FLAG_DELETED;
ret+=1; 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)
rb_hook_list_t *hooks = def->body.bmethod.hooks; rb_hook_list_t *hooks = def->body.bmethod.hooks;
VM_ASSERT(hooks != NULL); VM_ASSERT(hooks != NULL);
rb_hook_list_remove_tracepoint(hooks, tpval); rb_hook_list_remove_tracepoint(hooks, tpval);
if (hooks->running == 0) {
if (hooks->events == 0) {
rb_hook_list_free(def->body.bmethod.hooks); rb_hook_list_free(def->body.bmethod.hooks);
def->body.bmethod.hooks = NULL;
} }
def->body.bmethod.hooks = NULL;
} }
return ST_CONTINUE; return ST_CONTINUE;
} }
@ -1301,9 +1310,9 @@ rb_hook_list_remove_tracepoint(rb_hook_list_t *list, VALUE tpval)
while (hook) { while (hook) {
if (hook->data == tpval) { if (hook->data == tpval) {
hook->hook_flags |= RUBY_EVENT_HOOK_FLAG_DELETED; 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; events |= hook->events;
} }
hook = hook->next; hook = hook->next;