Skip to content

Run GC if fiber pool expansion fails.#16535

Merged
ioquatix merged 3 commits intoruby:masterfrom
samuel-williams-shopify:fiber-stack-acquire-gc
Mar 26, 2026
Merged

Run GC if fiber pool expansion fails.#16535
ioquatix merged 3 commits intoruby:masterfrom
samuel-williams-shopify:fiber-stack-acquire-gc

Conversation

@samuel-williams-shopify
Copy link
Contributor

@samuel-williams-shopify samuel-williams-shopify force-pushed the fiber-stack-acquire-gc branch 4 times, most recently from 337e10c to 5397956 Compare March 25, 2026 01:18
@launchable-app

This comment has been minimized.

@samuel-williams-shopify samuel-williams-shopify force-pushed the fiber-stack-acquire-gc branch 5 times, most recently from 7e61a36 to 8e4a9f5 Compare March 25, 2026 05:48
@ioquatix
Copy link
Member

I confirmed the failure path works on Linux.

@ioquatix ioquatix force-pushed the fiber-stack-acquire-gc branch 2 times, most recently from a1395e5 to fb42b3d Compare March 25, 2026 19:20
@samuel-williams-shopify samuel-williams-shopify changed the title Run GC if no fiber stacks are available, before expanding pool. Run GC if fiber pool expansion fails. Mar 25, 2026
@ioquatix ioquatix force-pushed the fiber-stack-acquire-gc branch 5 times, most recently from 5a4208b to e3b2b83 Compare March 25, 2026 23:42
@ioquatix ioquatix requested a review from Copilot March 26, 2026 01:27
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 addresses Bug #21964 by making fiber stack acquisition more resilient when the fiber pool cannot be expanded (e.g., due to VMA/map limits), triggering a GC + retry before ultimately raising.

Changes:

  • Refactors fiber_pool_expand to return NULL (with errno) on failure instead of raising immediately, allowing callers to decide recovery behavior.
  • Adds a slow-path in fiber_pool_stack_acquire to run rb_gc() and retry pool expansion when expansion fails.
  • Adds a regression test intended to exercise fiber creation under vm.max_map_count pressure.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
cont.c Changes fiber pool expansion/acquisition flow to GC+retry on expansion failure and adjusts error handling paths.
test/ruby/test_fiber.rb Adds a regression test to ensure fiber creation doesn’t raise when pool expansion hits mapping limits.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ioquatix ioquatix force-pushed the fiber-stack-acquire-gc branch from f1b938f to d15d96d Compare March 26, 2026 03:44
@samuel-williams-shopify samuel-williams-shopify force-pushed the fiber-stack-acquire-gc branch 2 times, most recently from 7bbb99e to 8efedc5 Compare March 26, 2026 06:22
Copy link
Member

@ioquatix ioquatix left a comment

Choose a reason for hiding this comment

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

LGTM.

@ioquatix ioquatix force-pushed the fiber-stack-acquire-gc branch from 746d84f to 2576c62 Compare March 26, 2026 08:24
@ioquatix ioquatix merged commit 7c8762a into ruby:master Mar 26, 2026
91 checks passed
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.

3 participants