fix(main/ruby): fix fork panicking
#23611
Open
+22
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
mbekkomo
commented
Mar 5, 2025
mbekkomo
commented
- Closes [Bug]: ruby 3.4.1 fork broken #23582
|
@csabahenk Could you test this after the build is done? |
|
It's interesting progress, but for me, this makes the error change from to |
|
This is one way to work around it I guess if there is no other way to be found, The change in ruby that caused the error to start appearing is this: ruby/ruby@63cbe3f Since the problem bisects to that commit, reverting it like this would cause the error to stop: Reverts https://github.com/ruby/ruby/commit/63cbe3f6ac9feb44a2e43b1f853e2ca7e049316c
which makes https://github.com/termux/termux-packages/issues/23582 go back to previous behavior (working)
--- a/ext/socket/raddrinfo.c
+++ b/ext/socket/raddrinfo.c
@@ -342,7 +342,7 @@ rb_getaddrinfo(const char *hostp, const char *portp, const struct addrinfo *hint
arg.service = portp;
arg.hints = hints;
arg.res = ai;
- return (int)(VALUE)rb_thread_call_without_gvl(fork_safe_getaddrinfo, &arg, RUBY_UBF_IO, 0);
+ return (int)(VALUE)rb_thread_call_without_gvl(nogvl_getaddrinfo, &arg, RUBY_UBF_IO, 0);
}
#elif GETADDRINFO_IMPL == 2
@@ -505,7 +505,7 @@ start:
}
pthread_t th;
- if (raddrinfo_pthread_create(&th, fork_safe_do_getaddrinfo, arg) != 0) {
+ if (raddrinfo_pthread_create(&th, do_getaddrinfo, arg) != 0) {
int err = errno;
free_getaddrinfo_arg(arg);
errno = err;
--- a/process.c
+++ b/process.c
@@ -4233,23 +4233,12 @@ rb_fork_ruby(int *status)
prefork();
before_fork_ruby();
- rb_thread_acquire_fork_lock();
disable_child_handler_before_fork(&old);
child.pid = pid = rb_fork();
child.error = err = errno;
disable_child_handler_fork_parent(&old); /* yes, bad name */
- if (
-#if defined(__FreeBSD__)
- pid != 0 &&
-#endif
- true) {
- rb_thread_release_fork_lock();
- }
- if (pid == 0) {
- rb_thread_reset_fork_lock();
- }
after_fork_ruby(pid);
/* repeat while fork failed but retryable */however, what the ruby creators are trying to do is make the language more thread-safe, so by completely reverting their change, this still does not "fully solve" the problem because it misses out on their feature, by continuing to use the old, less-thread-safe behavior that the old version of ruby was using. I think an ideal solution would probably be one that can properly fix the fork lock feature in a thread-safe way that does not have |
|
@mbekkomo alas, wiith ruby 3.4.1-1, ruby still panics, although in different function with a different errno. |
It's better to check the Ruby Issue Tracker, we might find something useful there to solve this problem. |
mbekkomo
changed the title
fork panicking on EPERMfork panicking because of thread deadlock
mbekkomo
changed the title
fork panicking because of thread deadlockfork panicking
|
@robertkirkman @csabahenk Can you try the latest commit build? |
|
I've tried the latest build myself and met with |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.