Skip to content

Conversation

@luke-gruber
Copy link
Contributor

@luke-gruber luke-gruber commented Nov 14, 2025

If we don't mark them, the thread object can be freed as soon as it ends. It doesn't seem to be a problem when the worker pool just has 1 thread, but if there are multiple threads then each call to Thread#join can cause a GC step as it calls back into ruby.

If you do this you'll get a pretty consistent crash:

class ::Thread
  alias_method :old_join, :join
  def join(timeout=nil)
    4.times { GC.start }
    old_join(timeout)
  end
end

If we don't mark them, the thread object can be freed as soon as it ends.
It doesn't seem to be a problem when the worker pool just has 1 thread,
but if there are multiple threads then each call to `Thread#join` can
cause a GC step as it calls back into ruby.

If you do this:

```
class ::Thread
  alias_method :old_join, :join
  def join(timeout=nil)
    4.times { GC.start }
    old_join(timeout)
  end
end
```

and set the size pool to be > 1, it can crash.
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, thanks for this excellent fix.

@ioquatix ioquatix force-pushed the main branch 3 times, most recently from ef2d35b to 44666dc Compare November 16, 2025 12:51
@ioquatix ioquatix merged commit 903522e into socketry:main Nov 16, 2025
45 of 70 checks passed
@ioquatix ioquatix added this to the v1.14.1 milestone Nov 16, 2025
nateberkopec added a commit to nateberkopec/io-event that referenced this pull request Nov 16, 2025
Commit 903522e contained git diff syntax that was accidentally committed
to the source file. This commit:

- Removes the errant git diff line (@@ -214,7 +227,7 @@)
- Restores proper tab indentation throughout create_worker_thread
- Ensures all worker struct fields are properly initialized

The functionality from PR socketry#152 (marking threads for GC) is preserved.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
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