Skip to content

Fix IO#close use-after-free in blocking_operation_wait and fiber_interrupt hooks. #13437

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.

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

Merged
merged 2 commits into from
Jun 6, 2025

Conversation

ioquatix
Copy link
Member

@ioquatix ioquatix commented May 24, 2025

This PR fixes memory safety and interrupt handling issues discovered while debugging IO#close across fibers.

Fix interrupt handling affecting rb_io_blocking_operation_exit

If rb_io_blocking_operation_exit executes with pending interrupts, user code that checks interrupts can exit unexpectedly. Instead, move the exception check into the EC_PUSH_TAG block so that by the time we reach rb_io_blocking_operation_exit the exception is already propagating and it behaves more like ensure.

Without this change, the following program can hang:

#!/usr/bin/env ruby

require_relative 'lib/async'

Async do
  r, w = IO.pipe
  
  read_thread = Thread.new do
    Thread.current.report_on_exception = false
    r.read(5)
  end
  
  # Wait until read_thread blocks on I/O
  Thread.pass until read_thread.status == "sleep"
  
  close_task = Async do
    r.close
  end
  
  close_task.wait
  begin
    read_thread.join
  rescue => error
    puts "Caught exception: #{error.class} - #{error.message}"
  end
end

This is a follow up to #12839

Fix use-after-free in blocking_operation_wait hook

IO#close could trigger stack-allocated memory access after the function returned. The new implementation uses a heap-allocated BlockingOperation instance which also exposes a C interface for (safe) native work pool execution.

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

Technical Implementation

The blocking operation system now provides:

  • Thread-safe cancellation with atomic state transitions
  • Cross-platform atomic operations supporting Windows, macOS, and Linux
  • Anonymous BlockingOperation class hidden from public API
  • Comprehensive state machine for operation lifecycle management
  • GC-safe object management with proper root registration

API Improvements

  • rb_fiber_scheduler_blocking_operation_extract - Safely extract operation data while holding GVL
  • rb_fiber_scheduler_blocking_operation_execute - Execute in thread pools without GVL concerns
  • rb_fiber_scheduler_blocking_operation_cancel - Thread-safe cancellation support
  • Consistent naming with RB_FIBER_SCHEDULER_BLOCKING_OPERATION_STATUS_* enum values

Testing

Enhanced tests covering:

  • IO#close across fibers
  • Memory safety verification
  • Blocking operation cancellation
  • Interrupt handling scenarios

@ioquatix ioquatix force-pushed the thread-io-check-interrupts branch from 2ebe626 to 5bccd06 Compare May 24, 2025 11:36

This comment has been minimized.

@ioquatix ioquatix force-pushed the thread-io-check-interrupts branch 5 times, most recently from 8b6b7cd to 20f5912 Compare May 25, 2025 01:38
@samuel-williams-shopify samuel-williams-shopify force-pushed the thread-io-check-interrupts branch from b7a00a6 to 5187cfe Compare May 29, 2025 02:04
@ioquatix ioquatix force-pushed the thread-io-check-interrupts branch from b602ba5 to f66e22f Compare May 29, 2025 21:43
@samuel-williams-shopify samuel-williams-shopify force-pushed the thread-io-check-interrupts branch 5 times, most recently from 94f3fbe to de20afa Compare May 30, 2025 19:43
@ioquatix ioquatix force-pushed the thread-io-check-interrupts branch 3 times, most recently from a49c5de to e57572a Compare June 5, 2025 01:28
@samuel-williams-shopify samuel-williams-shopify force-pushed the thread-io-check-interrupts branch 9 times, most recently from 02485cf to 9434e5c Compare June 5, 2025 06:51
@ioquatix ioquatix changed the title rb_io_blocking_operation_exit should not execute with pending interrupts. Fix IO#close use-after-free in blocking_operation_wait and fiber_interrupt hooks. Jun 5, 2025
@samuel-williams-shopify samuel-williams-shopify force-pushed the thread-io-check-interrupts branch 3 times, most recently from 7020636 to 26b796c Compare June 5, 2025 19:14
@samuel-williams-shopify samuel-williams-shopify force-pushed the thread-io-check-interrupts branch from 26b796c to 9647437 Compare June 5, 2025 21:11
@samuel-williams-shopify samuel-williams-shopify force-pushed the thread-io-check-interrupts branch 2 times, most recently from 0f6c53d to 106f80a Compare June 6, 2025 01:37
@ioquatix ioquatix force-pushed the thread-io-check-interrupts branch from 106f80a to a2cdc03 Compare June 6, 2025 02:26
@ioquatix ioquatix force-pushed the thread-io-check-interrupts branch from a2cdc03 to e03d153 Compare June 6, 2025 03:07
@ioquatix ioquatix merged commit ead14b1 into ruby:master Jun 6, 2025
82 checks passed
@ioquatix ioquatix deleted the thread-io-check-interrupts branch June 6, 2025 05:23
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.

2 participants