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

8233674: JarURLConnection.getJarFile throws Exception if some process is holding the file #5460

Closed
wants to merge 1 commit into from

Conversation

masyano
Copy link

@masyano masyano commented Sep 10, 2021

Could you please review the 8233674 bug fixes?
This problem is caused by the antivirus software opening the file for a short time, so CreateFile() should be retried.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-8233674: JarURLConnection.getJarFile throws Exception if some process is holding the file

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/5460/head:pull/5460
$ git checkout pull/5460

Update a local copy of the PR:
$ git checkout pull/5460
$ git pull https://git.openjdk.java.net/jdk pull/5460/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 5460

View PR using the GUI difftool:
$ git pr show -t 5460

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/5460.diff

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Sep 10, 2021

👋 Welcome back myano! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot added the rfr label Sep 10, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Sep 10, 2021

@masyano The following labels will be automatically applied to this pull request:

  • core-libs
  • net

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added core-libs net labels Sep 10, 2021
@mlbridge
Copy link

@mlbridge mlbridge bot commented Sep 10, 2021

Webrevs

Copy link
Contributor

@AlanBateman AlanBateman left a comment

This will need discussion as to whether the JDK should put in retry loops to work around interference from virus checkers. I wonder if it possible to get any input from the Microsoft engineers on whether native applications are expected to do this or whether the issue here is actually a configuration issue or problem with a specific virus checker. If we are adding a workaround to the JDK then it will require much more broader changes that does proposed so I think more analysis and discussion will be required before deciding whether it is right to do anywhere here or not.

@masyano
Copy link
Author

@masyano masyano commented Sep 17, 2021

Thank you for your comment. According to Microsoft KB316609, CreateFile() should be tried again a short time later. I think JarURLConnection should also be retried when antivirus software holds some files.
https://www.betaarchive.com/wiki/index.php/Microsoft_KB_Archive/316609

@AlanBateman
Copy link
Contributor

@AlanBateman AlanBateman commented Sep 17, 2021

Thank you for your comment. According to Microsoft KB316609, CreateFile() should be tried again a short time later. I think JarURLConnection should also be retried when antivirus software holds some files.
https://www.betaarchive.com/wiki/index.php/Microsoft_KB_Archive/316609

This seems to be just general advice about dealing with sharing violations. I think it would be useful to hear from the Microsoft folks on whether every application on Windows is expected to retry to workaround interference from virus checkers or whether this is something that should be fixed by the virus checker vendors. If the JDK ends up putting in a workaround then it will require more broader changes, things will work inconsistently if limited to opening zip files and java.io.

@masyano
Copy link
Author

@masyano masyano commented Sep 27, 2021

It’s a good idea to ask the Microsoft folks about that, but I don't know the way to ask. Could you tell me how to do it?

As you say, CreateFile function is used in other parts of the JDK, but it have a significant impact to fix all of that. Therefore, I fixed the problem about opening zip files and jar files which is reported in the JBS.

@AlanBateman
Copy link
Contributor

@AlanBateman AlanBateman commented Sep 27, 2021

It’s a good idea to ask the Microsoft folks about that, but I don't know the way to ask. Could you tell me how to do it?

As you say, CreateFile function is used in other parts of the JDK, but it have a significant impact to fix all of that. Therefore, I fixed the problem about opening zip files and jar files which is reported in the JBS.

I think we need to find out if the issue you are working around is a bug/issue with the virus checker or that Microsoft recommends that all applications should put a retry to work around these issues. As regards the patch then it is incomplete. If we are forced to put a workaround into the JDK code then I think it will have to do everywhere, not just for zip/JAR files.

@masyano
Copy link
Author

@masyano masyano commented Oct 6, 2021

I inquired of the Microsoft Technical Support about this problem. They said that using a simple retry mechanism is the effective workaround for this problem as in the article. But, they are not sure which is wrong the application or the virus checker because it depends on the situation. Which method do you think is better, to fix the JDK or to change the configuration of the virus checker?

@masyano
Copy link
Author

@masyano masyano commented Oct 18, 2021

@AlanBateman Could you reply to the above comment?

@AlanBateman
Copy link
Contributor

@AlanBateman AlanBateman commented Oct 18, 2021

@AlanBateman Could you reply to the above comment?

I view this issue as something for the anti-virus vendor to fix. It would be very disappointing to force the JDK to put in a workaround for this. A workaround would be very intrusive and require changes in many areas of the JDK.

@masyano
Copy link
Author

@masyano masyano commented Oct 28, 2021

Thank you for your reply.
I understand we have no need to fix the JDK. I will close this pull request.

@masyano masyano closed this Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core-libs net rfr
2 participants