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

More robust way to detect atomic move failure. Fixes #5415. #5416

Closed
wants to merge 0 commits into from

Conversation

headius
Copy link
Member

@headius headius commented Nov 6, 2018

It must have escaped my notice when improving the EXDEV error propagation that the JDK does have a specific error type for failed atomic moves. This logic should work on all OpenJDK 7+ based JDKs.

@ahorek If you can confirm this works for you locally (I have confirmed it myself on MacOS) it would be a big help!

@headius headius added this to the JRuby 9.2.1.0 milestone Nov 6, 2018
@ahorek
Copy link
Contributor

ahorek commented Nov 6, 2018

confirmed! thanks

@headius
Copy link
Member Author

headius commented Nov 6, 2018

@ahorek @enebo I have pushed two additional commits that add more java.io and java.nio.file exceptions to this more robust logic. We can add those, or defer them if it's too much of a risk. The original commit should get cherry-picked in if that's the case.

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.

I think we should just put the EXDEV fix in this alone for 9.2.1.0 and then put the rest in after release. All our other string matches have been working for years without complaint and I don't want to add another variable of uncertainty for 9.2.1.0. 9.2.2.0 can hopefully have a little bake and we can have some confidence that the rest of this is solid (which I suspect it is but I am a bit paranoid this week).

@headius headius mentioned this pull request Nov 6, 2018
@headius headius closed this Nov 6, 2018
@headius headius deleted the fix_exdev_error branch November 6, 2018 15:44
@headius
Copy link
Member Author

headius commented Nov 6, 2018

I moved the other two commits to #5417 and merged in just the EXDEV fix for 9.2.1.

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

3 participants