Skip to content

Allow IO#close to interrupt IO operations on fibers using fiber_interrupt hook. #12839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
ioquatix merged 3 commits into from
May 23, 2025
Merged

Allow IO#close to interrupt IO operations on fibers using fiber_interrupt hook. #12839

ioquatix merged 3 commits into from
May 23, 2025
+328 −24

Conversation

@ioquatix
Copy link
Member

@ioquatix ioquatix commented Mar 3, 2025 •
edited
Loading

Ruby's IO#close can cause IO#read, IO#write, IO#wait, IO#wait_readable and IO#wait_writable to be interrupted with an IOError (stream closed on another thread).

The fiber scheduler did not implement this for io_read, io_write and io_wait hooks. Finally after several years, someone made a bug report - see socketry/async#368 for background. In order to solve this problem for the fiber scheduler, we need to expose the ability for IO#close to interrupt a fiber.

This PR introduces a new internal C function rb_thread_io_blocking_operation which takes an IO instance and a callback. During that callback, if the IO is closed, the callback will be interrupted.

It also introduces a new fiber scheduler method, fiber_interrupt which allows us to interrupt a specific fiber with an error, in this case with an IOError.

https://bugs.ruby-lang.org/issues/21166

@launchable-app
Copy link

launchable-app bot commented Mar 3, 2025 •
edited
Loading

❌ Tests Failed

✖️no tests failed ✔️61866 tests passed(1 flake)

@ioquatix ioquatix force-pushed the fiber-scheduler-fiber_interrupt-internal branch 5 times, most recently from ee34fe8 to 56d24c9 Compare March 3, 2025 08:37
luke-gru
luke-gru reviewed Mar 3, 2025
View reviewed changes
@luke-gru
Copy link
Contributor

luke-gru commented Mar 3, 2025 •
edited
Loading

For what it's worth this patch makes sense and LGTM, but I have no experience using the fiber scheduler and this needs another reviewer. But thank you for making the changes I suggested 😄

@ioquatix ioquatix mentioned this pull request Mar 10, 2025
Closed
@ioquatix ioquatix changed the title Implement interruptible IO within the scheduler hook interface. Allow IO#close to interrupt IO operations on fibers using fiber_interrupt hook. Mar 10, 2025
@ioquatix ioquatix force-pushed the fiber-scheduler-fiber_interrupt-internal branch 4 times, most recently from 4836882 to b69bafb Compare March 11, 2025 06:12
@ioquatix ioquatix mentioned this pull request Apr 18, 2025
Closed
@ioquatix ioquatix force-pushed the fiber-scheduler-fiber_interrupt-internal branch 6 times, most recently from cb63a82 to 5694eb1 Compare May 13, 2025 13:42
@ioquatix ioquatix requested a review from ko1 May 13, 2025 13:42
@ioquatix ioquatix force-pushed the fiber-scheduler-fiber_interrupt-internal branch from 5694eb1 to 0c48c46 Compare May 13, 2025 13:52
@ioquatix ioquatix requested a review from Copilot May 13, 2025 13:55
Copilot AI reviewed May 13, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new C function to perform interruptible IO operations in the fiber scheduler, allowing IO#close to interrupt fibers and raise an IOError during blocking operations. Key changes include the implementation of rb_thread_io_blocking_operation with its ensure routine, integration of a new Fiber::Scheduler#fiber_interrupt hook for interrupting fibers, and updated IO buffering and scheduler functions to use the new interruptible operations.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
thread.c Added new functions for interruptible IO operations and modified return type conversion in IO close.
test/fiber/scheduler.rb Added exception handling for IO.select and introduced FiberInterrupt and fiber_interrupt functions.
scheduler.c Added new scheduler methods that wrap IO methods with the new interruptible blocking operation.
io_buffer.c Updated read/write/pwrite functions to correctly resolve IO instances before performing operations.
internal/thread.h Declared the new rb_thread_io_blocking_operation function along with updated comments.
include/ruby/fiber/scheduler.h Updated documentation for the new fiber_interrupt API.
NEWS.md Documented the addition of Fiber::Scheduler#fiber_interrupt for better developer visibility.

@ioquatix ioquatix force-pushed the fiber-scheduler-fiber_interrupt-internal branch 3 times, most recently from 42b1f72 to 2239ea0 Compare May 15, 2025 10:54
ioquatix
ioquatix commented May 17, 2025
View reviewed changes
@ioquatix ioquatix force-pushed the fiber-scheduler-fiber_interrupt-internal branch from 411f214 to 9a1a3ce Compare May 22, 2025 07:57
@ioquatix ioquatix force-pushed the fiber-scheduler-fiber_interrupt-internal branch from 9a1a3ce to 872afdd Compare May 23, 2025 00:08
ko1
ko1 reviewed May 23, 2025
View reviewed changes
ko1
ko1 reviewed May 23, 2025
View reviewed changes
@ioquatix
Copy link
Member Author

ioquatix commented May 23, 2025

Benchmark results:

> make benchmark BENCH_RUBY=/Users/samuel/.rubies/ruby-head/bin/ruby COMPARE_RUBY=/Users/samuel/.rubies/ruby-a6435bef/bin/ruby ITEM=io_close
/Users/samuel/.rubies/ruby-3.4.2/bin/ruby --disable=gems -rrubygems -I./benchmark/lib ./benchmark/benchmark-driver/exe/benchmark-driver \
	            --executables="compare-ruby::/Users/samuel/.rubies/ruby-a6435bef/bin/ruby -I.ext/common --disable-gem" \
	            --executables="built-ruby::/Users/samuel/.rubies/ruby-head/bin/ruby --disable-gem" \
	            --output=markdown --output-compare -v $(find ./benchmark -maxdepth 1 -name 'io_close' -o -name '*io_close*.yml' -o -name '*io_close*.rb' | sort) 
compare-ruby: ruby 3.5.0dev (2025-05-13T08:35:34Z master a6435befa7) +PRISM [arm64-darwin24]
built-ruby: ruby 3.5.0dev (2025-05-17T00:42:12Z master aa0f689bf4) +PRISM [arm64-darwin24]
last_commit=[ruby/net-http] Freeze some constants to improve Ractor compatibility
# Iteration per second (i/s)

|                    |compare-ruby|built-ruby|
|:-------------------|-----------:|---------:|
|io_close_contended  |     117.394|   179.029|
|                    |           -|     1.53x|
|io_close            |     821.862|   900.114|
|                    |           -|     1.10x|

@ioquatix ioquatix merged commit 73c9d6c into ruby:master May 23, 2025
82 checks passed
@ioquatix ioquatix deleted the fiber-scheduler-fiber_interrupt-internal branch May 23, 2025 05:55
@ioquatix ioquatix mentioned this pull request May 24, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ko1 ko1 ko1 left review comments

@luke-gru luke-gru luke-gru left review comments

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ioquatix @luke-gru @ko1