Skip to content
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

Fix spec of Thread#status for an externally killed thread that sleeps #4815

Merged
merged 1 commit into from
Oct 12, 2017

Conversation

iaddict
Copy link
Contributor

@iaddict iaddict commented Oct 12, 2017

Fixes #4705

Copy link
Member

@enebo enebo left a comment

Choose a reason for hiding this comment

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

On the face of this I think this seems reasonable but I think @headius should also review this since it is threading and threading is hard :)

@headius
Copy link
Member

headius commented Oct 12, 2017

There could be a race here, if a thread marks itself as aborting and then some other thread tries to exit sleep at the same time, but ideally exitSleep should only be called by that thread when it transitions. The uses I reviewed seem to fit that pattern.

Worst case, some bad actor might cause an aborting thread to mark itself as runnable again. it would be no worse than now but less likely to happen.

@headius headius merged commit 6dd82b3 into jruby:master Oct 12, 2017
@headius headius added this to the JRuby 9.2.0.0 milestone Oct 12, 2017
@eregon
Copy link
Member

eregon commented Oct 12, 2017

FWIW The way we do in in TruffleRuby is to save/restore the Thread status around sleep/blocking tasks.
And since only the current thread modifies its status and we poll for interrupts after restoring the thread state when interrupted in a blocking task, I believe this is safe and race-free.

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.

None yet

4 participants