Bug #19172


`ruby_thread_has_gvl_p` is innacurate sometimes -- document or change?

Added by ivoanjo (Ivo Anjo) 6 months ago. Updated 4 months ago.

Target version:
ruby -v:
(All Ruby versions)


Howdy 👋! I work for Datadog on the ddtrace gem 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.


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

    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, 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?

Actions #1

Updated by ivoanjo (Ivo Anjo) 6 months ago

  • Description updated (diff)

Updated by luke-gru (Luke Gruber) 4 months ago

In doing some reading of the code, it's my understanding that (TH_SCHED(th)->running == th) would be a better way to tell if a thread has the GVL. Maybe I'm understanding it wrong though.

Updated by ivoanjo (Ivo Anjo) 4 months ago

Yeah, that's my understanding, and what I'm using in that PR (although with a lot more complexity since I'm still trying to support older Rubies...).


Also available in: Atom PDF