Skip to content

Use parent thread for Thread.current within a fiber - #5827

Closed
headius wants to merge 1 commit into
masterfrom
spooky_fibers
Closed

headius wants to merge 1 commit into
masterfrom
spooky_fibers

Conversation

@headius

@headius headius commented Aug 10, 2019

Copy link
Copy Markdown
Member

This is the start of experiments in making Thread.current, among other things, pretend that a Fiber is equivalent to its master thread, rather than being its own thread.

This work was triggered by me running the socketry/async specs and seeing that there's some locking done across fibers that's expected to treat them all as owning the lock. In this case, it's
an RSpec reentrant lock implementation that aggregates a Mutex and the thread that acquired it and compares that thread with Thread.current to avoid relocking.

For JRuby the following changes were needed to make the lock work:

  • Thread.current must point at the "Fiber current thread" rather than the "Fiber thread". We have attempted this in the past and run afoul of system-level locking that saw through to the real
    locks.
  • Mutex#owned? had to do a similar check, aggregating a reference to the Fiber current thread that originally locked it and using that for comparison with the current Fiber current thread.

With these changes the async specs run to completion with 16 errors that I have not looked into yet.

See #1806.

@headius headius added this to the JRuby 9.3.0.0 milestone Aug 10, 2019
@headius headius modified the milestones: JRuby 9.3.0.0, JRuby 9.3.1.0 Jul 1, 2021
@headius headius modified the milestones: JRuby 9.3.1.0, JRuby 9.4.0.0 Oct 7, 2021
@headius

headius commented Oct 7, 2021

Copy link
Copy Markdown
Member Author

This didn't make the cut for 9.3, which we wanted to get out the door to work on Ruby 3.0 support in 9.4. Punting to 9.4 to reevaluate.

@headius

headius commented Nov 4, 2022

Copy link
Copy Markdown
Member Author

Rebased on master. With recent changes to fibers, we'll see how this goes in CI. It's not something I'd like to drop in an update release, so if we are green with this change now might be the time to push it through.

This is the start of experiments in making Thread.current, among
other things, pretend that a Fiber is equivalent to its master
thread, rather than being its own thread.

This work was triggered by me running the socketry/async specs and
seeing that there's some locking done across fibers that's
expected to treat them all as owning the lock. In this case, it's
an RSpec reentrant lock implementation that aggregates a Mutex and
the thread that acquired it and compares that thread with
Thread.current to avoid relocking.

For JRuby the following changes were needed to make the lock work:

* Thread.current must point at the "Fiber current thread" rather
than the "Fiber thread". We have attempted this in the past and
run afoul of system-level locking that saw through to the real
locks.
* Mutex#owned? had to do a similar check, aggregating a reference
  to the Fiber current thread that originally locked it and using
that for comparison with the current Fiber current thread.

With these changes the async specs run to completion with 16
errors that I have not looked into yet.
@headius

headius commented Nov 16, 2022

Copy link
Copy Markdown
Member Author

There appears to be several hanging tests, so something is not right here.

@headius

headius commented Aug 31, 2026

Copy link
Copy Markdown
Member Author

Going to reboot this on a separate branch and PR.

@headius headius closed this Aug 31, 2026
headius added a commit to headius/jruby that referenced this pull request Aug 31, 2026
A few step toward getting Thread.current in general to behave like
CRuby, returning the same Thread for all Fiber instances that it
owns.

Behind the scenes, each Fiber is still backed by a VirtualThread
with an associated ThreadContext and RubyThread, but user code
should generally not see those.

Additional fixes will follow for internal code that has
expectations about the current thread of a fiber.

See other attempts to do this and issues related:

* My attempt: jruby#5827
* Community attempt: jruby#1225
* An early issue: jruby#1806

Relates to test failures mentioned in jruby#9603 and
workarounds from users in jruby#9600.
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.

1 participant