Bug #19172
Updated by ivoanjo (Ivo Anjo) over 1 year ago
Howdy 👋! I work for Datadog [on the ddtrace gem](https://github.com/DataDog/dd-trace-rb) and I found a... sharp edge on the internal `ruby_thread_has_gvl_p` API. I am aware that `ruby_thread_has_gvl_p` is documented an experimental API that is exported as a symbol but not present on the VM include files. ### Background In the ddtrace profiling component, we setup a signal handler and then periodically send SIGPROF signals to try to interrupt the running Ruby thread (e.g. the thread that is holding the global VM lock or equivalent). In the signal handler, we need to perform some API calls which are not safe to do without the GVL. So we need to check if the signal handler got called in the thread that has the GVL. ### The issue ```c ```ruby int ruby_thread_has_gvl_p(void) { rb_thread_t *th = ruby_thread_from_native(); if (th && th->blocking_region_buffer == 0) { return 1; } else { return 0; } } ``` In its current implementation, `ruby_thread_has_gvl_p` only checks if the thread has a `blocking_region_buffer` or not. Unfortunately, this means that when called from a thread that lost the GVL but not due to blocking (e.g. via `rb_thread_schedule()`), it can still claim that a thread is holding the GVL when that is not the case. I ran into this issue in https://github.com/DataDog/dd-trace-rb/pull/2415, and needed to find a workaround. ### Next steps Since this is an internal VM API, I'm not sure you'd want to change the current behavior, so I was thinking of perhaps two options: * Is it worth changing `ruby_thread_has_gvl_p` to be accurate in the case I've listed? * If not, would you accept a PR to document its current limitations, so that others don't run into the same issue I did?