Bug #20863
Updated by ioquatix (Samuel Williams) 12 days ago
## Background I was working on https://bugs.ruby-lang.org/issues/20876 and was investigating some problems with `zlib.c` and GVL, and noticed that `zstream_run_func` is executed without it releases the GVL, but can invoke still calls various `rb_` string functions. Those functions in turn can invoke `rb_raise` and generally look problematic. However, maybe by luck, such code path does not appear According to be invoked my understanding, this is invalid. I propose the following fix: https://github.com/ruby/zlib/pull/88 In addition, I think we should add more checks for this, in typical usage. However, even so, it is possible other words, we should add something like this to cause `zstream_run_func` to segfault by a carefully crafted program functions which causes the internal buffer are expected to only be resized called while the GVL is released: https://github.com/ruby/zlib/pull/88#issuecomment-2455772054 held: ## Proposal ```c RUBY_ASSERT(ruby_thread_has_gvl_p()) ``` I would like made a simple PR to modify `zlib.c` to only release demonstrate the GVL around change: https://github.com/ruby/ruby/pull/11975 While the CPU intensive compression/decompression operation: https://github.com/ruby/zlib/pull/88 In addition, current implementation is internal only (`ruby_thread_has_gvl_p` is not public interface), I recommend think we introduce additional checks should add this interface as a public macro, e.g. `RUBY_DEBUG_ASSERT_GVL` or something like that. This macro should be added to prevent this class of bug from occurring all methods that require the GVL in again: https://bugs.ruby-lang.org/issues/20877 order to diagnose these issues. Some ideas for public macro name: - `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_LOCKED`