Feature #20877
Updated by ioquatix (Samuel Williams) 15 days ago
## Background I found issues with `zlib.c` calling `rb_` functions without holding the GVL: <https://bugs.ruby-lang.org/issues/20863>. https://bugs.ruby-lang.org/issues/20863 ## Background The GVL must be held before many of Ruby's C functions can be called. However, few functions check enforce this. As such, bugs like this may exist in other parts of the code and native extensions. Even thought it may work in many scenarios, it may break unexpectedly (SEGFAULT etc). ## Proposal ### Introduce more checks in CRuby I think we should (1) add more debug checks within CRuby for detecting this invalid usage. usage and (2) expose a public macro that can be used by Ruby's native extensions so that they may do the same. The current internal implementation looks like this: ```c RUBY_ASSERT(ruby_thread_has_gvl_p()); ``` While fixing `zlib.c`, I made a simple PR to demonstrate the change: https://github.com/ruby/ruby/pull/11975 ### Introduce a public macro for this check I think we should expose a public macro that can be used by Ruby's native extensions so that they may do the same check. `ruby_thread_has_gvl_p` is not a public interface at this time, so we can't use it in native extensions. I would like to consider adding a public macro to allow for this, e.g. `RUBY_DEBUG_ASSERT_GVL` or something similar. Finally, I propose to add this macro to all methods that should be executed with the GVL so that we catch invalid usage. If the name `RUBY_DEBUG_ASSERT_GVL` is not suitable / too specific, maybe some other ideas: - `RUBY_DEBUG_ASSERT_THREAD` - `RUBY_DEBUG_ASSERT_CURRENT_THREAD` or `RUBY_DEBUG_ASSERT_CURRENT_THREAD_P` - `RUBY_DEBUG_ASSERT_GVL` or `RUBY_DEBUG_ASSERT_GVL_P` - `RUBY_DEBUG_ASSERT_INTERPRETER_LOCKED` I don't have a strong opinion on naming, but I have a strong opinion about preventing invalid usage.