Skip to content

call rb_warn() if fail to create timer_thread, set system_working=0 #13671

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

luke-gruber
Copy link
Contributor

We should check this in general, but there are specific cases that worry me about timer thread creation.

In rb_fork_ruby(), if the call to fork fails, we handle the error (sleep or GC depending on errno), and try again. If fork fails because of resource issues, we can't assume the call in after_fork_ruby() to create the timer thread will succeed. If it fails, we get undefined behavior in the next iteration of the loop when we call pthread_join() on it in before_fork_ruby().

@ko1
Copy link
Contributor

ko1 commented Jun 20, 2025

it is better to run without timer thread even if it has an issue.
How about to warn about that?

@luke-gruber
Copy link
Contributor Author

luke-gruber commented Jun 20, 2025

Okay, that makes sense. I'll make sure not call pthread_join on it in next iteration if fork fails and timer thread fails. I'll also warn the user.

Edit: I updated the PR. The failed CI run is due to a flake test.

@luke-gruber luke-gruber force-pushed the rb_bug_if_cant_create_timer_thread branch from acd3477 to 7132323 Compare June 20, 2025 17:43
@luke-gruber luke-gruber changed the title call rb_bug() if we fail to create timer_thread call rb_warn() if fail to create timer_thread, set system_working=0 Jun 20, 2025
@luke-gruber luke-gruber force-pushed the rb_bug_if_cant_create_timer_thread branch from 7132323 to 339e4ea Compare June 20, 2025 17:45
We should do this in general, but there are specific cases that worry
me about timer thread creation.

In `rb_fork_ruby()`, if the call to `fork` fails, we handle the error
(sleep or GC depending on errno), and try again. If fork fails because
of resource issues, we can't assume the call in `after_fork_ruby()` to
create the timer thread will succeed. If it fails, we get undefined
behavior in the next iteration of the loop when we call pthread_join()
on it in `before_fork_ruby`. To fix this, we set `system_working = 0` if
pthread creation fails.
@luke-gruber luke-gruber force-pushed the rb_bug_if_cant_create_timer_thread branch from 339e4ea to 0bcf056 Compare June 20, 2025 17:47
Copy link

launchable-app bot commented Jun 20, 2025

Tests Failed

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

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