Skip to content

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

Closed
headius wants to merge 0 commit intojruby:masterfrom
headius:fix_exdev_error
Closed

More robust way to detect atomic move failure. Fixes #5415.#5416
headius wants to merge 0 commit intojruby:masterfrom
headius:fix_exdev_error

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.

3 participants