Skip to content

gvl: release GVL during streaming I/O, re-acquire for Ruby block yields#82

Open
ruyrocha wants to merge 2 commits into
SearchApi:mainfrom
ruyrocha:feat/release-gvl
Open

gvl: release GVL during streaming I/O, re-acquire for Ruby block yields#82
ruyrocha wants to merge 2 commits into
SearchApi:mainfrom
ruyrocha:feat/release-gvl

Conversation

@ruyrocha
Copy link
Copy Markdown

Previously, chunks used Yield::Iter over a BodyReceiver iterator. BodyReceiver::next called maybe_block_on, which held the GVL across the entire Tokio future — blocking all other Ruby threads for the duration of each network wait.

Replace with a Rust-driven loop inside nogvl_cancellable:

  • GVL is released while polling the async byte stream (network I/O)
  • GVL is re-acquired via with_gvl + block_in_place only to yield each chunk to the Ruby block
  • Thread interruption via Thread.kill is handled at every iteration step through the existing cancellation flag
  • Streaming errors now propagate as Ruby exceptions instead of being silently swallowed by the and_then(|r| r.ok()) in the old iterator

Add gvl::with_gvl as the counterpart to nogvl/nogvl_cancellable, add rt::runtime() to expose the global Tokio handle, and remove rt::maybe_block_on which is no longer needed.

The Ruby block Proc is pinned against GC for the duration of streaming via rb_gc_register_address.

Fixes #57

ruyrocha added 2 commits May 27, 2026 19:13
Previously, `chunks` used `Yield::Iter` over a `BodyReceiver` iterator.
`BodyReceiver::next` called `maybe_block_on`, which held the GVL across
the entire Tokio future — blocking all other Ruby threads for the duration
of each network wait.

Replace with a Rust-driven loop inside `nogvl_cancellable`:
- GVL is released while polling the async byte stream (network I/O)
- GVL is re-acquired via `with_gvl` + `block_in_place` only to yield
  each chunk to the Ruby block
- Thread interruption via Thread.kill is handled at every iteration
  step through the existing cancellation flag
- Streaming errors now propagate as Ruby exceptions instead of being
  silently swallowed by the `and_then(|r| r.ok())` in the old iterator

Add `gvl::with_gvl` as the counterpart to `nogvl`/`nogvl_cancellable`,
add `rt::runtime()` to expose the global Tokio handle, and remove
`rt::maybe_block_on` which is no longer needed.

The Ruby block Proc is pinned against GC for the duration of streaming
via `rb_gc_register_address`.

Fixes SearchApi#57
Replace the placeholder gvl_streaming_test.rb with a full regression and
correctness suite covering every behaviour changed or fixed in this PR.

New test groups:

  Basic functionality
  - Yield count, String type, binary encoding, non-empty chunks
  - nil return value regardless of block return
  - Empty body (204), single-chunk total bytes
  - LocalJumpError without block

  Block control flow (new)
  - break stops iteration cleanly without raising
  - next continues iteration, skipping only the current block body
  - Exception class and message preserved through the Rust loop
  - Iteration stops at the correct chunk when block raises

  Body ownership (extended)
  - bytes/text after chunks raises MemoryError (both directions)
  - GC registration leak regression: forced GC after double-consume
    must not corrupt the heap (validates the GcGuard fix)

  Content integrity (extended)
  - Streamed bytes match buffered bytes for same endpoint
  - 256 KB large body total size
  - 20-chunk stream all received

  GVL correctness (new/improved)
  - Background ticker thread accumulates > 10 ticks during a 3-second
    drip; uses Array#push instead of bare integer to avoid data race
  - Mutex starvation regression (SearchApi#57): a waiter thread must acquire a
    Mutex before streaming completes — the pre-fix BodyReceiver held
    the GVL continuously, starving all other threads
  - Concurrent streams: same client, different clients (3 threads)

  Thread interruption (improved)
  - Thread.kill during network wait: sets `started` flag before killing
    so the test cannot pass vacuously if get() itself fails
  - Thread.kill during block execution (sleeping inside the block)
  - Thread.raise: injected exception class must be preserved end-to-end

  Streaming error propagation (new)
  - Timeout mid-stream raises a typed Wreq error
  - Error is not silently swallowed as EOF (pre-fix regression)

  GC safety (extended)
  - Forced full GC + GC.compact between every chunk
  - Separate compaction test (skipped if GC.compact unavailable)
  - Aggressive GC during 8 KB stream must not corrupt content

  close() integration
  - close after full stream
  - close after partial stream (break mid-way)

  Client variants
  - Module method (Wreq.get)
  - Client instance
  - POST response
@0x676e67
Copy link
Copy Markdown
Collaborator

0x676e67 commented May 27, 2026

No GVL is held during the maybe_block_on call; it only releases the GVL.
You’re correct about one thing: and_then(|r| r.ok()) swallows timeout errors, which definitely needs a proper solution. However, iterators don’t natively support returning a Result under normal circumstances — only Option is possible.

@0x676e67
Copy link
Copy Markdown
Collaborator

This PR is not related to the issue at https://github.com/SearchApi/wreq-ruby/issues/57.
Our intention is to access the Ruby GVL within Rust threads to iterate over the user's request stream Body, but what is being implemented here is for the Response Body.

@ruyrocha
Copy link
Copy Markdown
Author

@0x676e67 It seems they're going to be two separate issues.

I'll drop the Fixes #57 reference and reopen this as a standalone response streaming fix. I'd then like to take a proper pass at the actual issue allowing Rust threads to drive iteration of a Ruby-side request body via the GVL. Before I do that: is the expectation that the caller passes an Enumerator, any object responding to each, or something else? And should Rust pull from it synchronously per-chunk or register a callback?

@0x676e67
Copy link
Copy Markdown
Collaborator

ruyrocha

Yes, but we're putting this issue on hold for now (it doesn't seem easy to implement; it would require embedding the Ruby interpreter and handling interpreter initialization). However, PRs are welcome. We’ll only revisit this when we seriously consider releasing v2.

@0x676e67
Copy link
Copy Markdown
Collaborator

A quick hint: if you've ever worked with PyO3, you'll understand the details. GVL is analogous to GIL—we also need to avoid blocking Rust async threads with GVL during iteration. You can check this out:
https://github.com/0x676e67/wreq-python/blob/58751c59bf00b2bf30b4b8ebb2dbe386348df59d/src/client/body/stream.rs#L213

@ruyrocha
Copy link
Copy Markdown
Author

@0x676e67 I never used PyO3 before :)

I think the main gotcha here is around the differences between GVL & GIL, and no equivalent APIs. Instead of trying to acquire the GVL from a Tokio thread, spawn a genuine Ruby thread via rb_thread_create(). This thread naturally holds the GVL and can call Ruby APIs. It releases the GVL only when waiting for channel capacity (so other Ruby threads can run), and re-acquires it for the next .next call

@0x676e67
Copy link
Copy Markdown
Collaborator

@0x676e67 I never used PyO3 before :)

I think the main gotcha here is around the differences between GVL & GIL, and no equivalent APIs. Instead of trying to acquire the GVL from a Tokio thread, spawn a genuine Ruby thread via rb_thread_create(). This thread naturally holds the GVL and can call Ruby APIs. It releases the GVL only when waiting for channel capacity (so other Ruby threads can run), and re-acquires it for the next .next call

That's a solid idea. Magnus should provide APIs for this. We generally avoid using rb-sys unless absolutely necessary, as it contains numerous unsafe APIs and requires extreme caution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

V2: Allows Rust threads to handle Ruby stream iteration using GVL

2 participants