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

Remove ZipError usage which is dead code since JDK 9 (JDK-8336843). #7862

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

mbien
Copy link
Member

@mbien mbien commented Oct 10, 2024

  • CachingArchive catches Exception from now on which seems to be the safest approach
  • removed the ZipError handler from NexusRepositoryIndexerImpl since the section is inside a repo mutex
  • JDK might remove the class itself at some point, see https://bugs.openjdk.org/browse/JDK-8336843 or the linked issue

closes #7818

@mbien mbien added Code cleanup Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) Maven [ci] enable "build tools" tests Upgrade JDK Upgrade to the JDK requirements of a module. labels Oct 10, 2024
@mbien mbien added this to the NB24 milestone Oct 10, 2024
Copy link
Contributor

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

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

In general I agree with the direction. For the NexusRepositoryIndexerImpl, it might be better to also catch Exception. Reasoning: I don't see how the code could have raised an ZipError from the storage operation, but it could raised because the maven indexer fails to index the ZIP/JAR. That could happen independently from holding the repository mutex.

@mbien
Copy link
Member Author

mbien commented Oct 11, 2024

I take a look if I can improve the exception handling there. The MutexException, which represents all exceptions which can occur inside the mutex, is already caught a few lines later and directly sent to the exception window. The block inside the mutex doesn't really handle any Exception, even though everything inside of maven-indexer is mapped to IOEs.

The ZipError had to be caught separately, since my guess is that errors might not be mapped to MutexException.

 - CachingArchive catches Exception from now on which seems to be
   the safest approach
 - removed the ZipError handler from NexusRepositoryIndexerImpl since
   the section is inside a repo mutex and MutexExceptions are already
   handled
@mbien mbien force-pushed the remove-ziperror-usage branch from 52754e7 to 88475d3 Compare October 11, 2024 02:04
Copy link
Contributor

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

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

Ah I see now. The magic is in org.openide.util.Mutex#writeAccess which catches the orignal exception and wraps it into a MutexException, that is then caught.

If I'm not totally misreading the JDK (or my grep skills went into the weekend before me) ZipError was already not thrown anymore since at least JDK 11, so this should be safe.

@mbien
Copy link
Member Author

mbien commented Oct 11, 2024

If I'm not totally misreading the JDK (or my grep skills went into the weekend before me) ZipError was already not thrown anymore since at least JDK 11, so this should be safe.

yep, since the point when the zip impl switched from native to java which was JDK 9, see JDK-8145260, JDK-8336843

thanks for the review, merging

@mbien mbien merged commit a1a87f5 into apache:master Oct 11, 2024
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code cleanup Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) Maven [ci] enable "build tools" tests Upgrade JDK Upgrade to the JDK requirements of a module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CachingArchive.java and NexusRepositoryIndexerImpl.java references the java.util.zip.ZipError class
2 participants