Community
Participate
Working Groups
Created attachment 81993 [details] Test program to reproduce the bug Build ID: 3.3.0 but also on 3.3.1 Steps To Reproduce: The bug leads to failed installations of WTP using the update manager on VMWare Images on fast machines with lower clock resolution (15ms instead of 10ms). It may also happen on fast PCs but I have never seen this happen. It is hard to reproduce, therefore I created a test program by copying the source code of the method from the plugin org.eclipse.update.core. 1. Run the attached test program (add to simple Java project) 2. It prints out 'failed to create file' messages for the org. version of getLocalRandomIdentifier() 3. It doesn't print out 'failed to create file' messages for the fixed version of getLocalRandomIdentifier() 4. In the temp. dir UpdMgrTest you don't find 10 feature.xml files 5. In the temp. dir UpdMgrTestFixed you find 10 feature.xml files. More information: ErrorRecoveryLog.getLocalRandomIdentifier() should create unique filename for every call. It uses a timestamp to make the filename unique and implements a check in file system to see that the file is really unique since clock resolution is not sufficient (e.g. 10 ms on Windows, worse on VMWare images). This check is buggy. Fix: Plugin org.eclipse.update.core Class: org.eclipse.update.internal.core.ErrorRecoveryLog Method: getLocalRandomIdentifier(String path) Line 98 while (new File(newName).exists()) { must be replaced by while (new File(file.getParentFile(), newName).exists()) {
Increased severity. Install and update scenarios can't be tested on our standard test infrastructure. It is simple to fix. Any chance for 3.3.2?
Can you explain your fix a bit more and provide a patch? We have no developers for Update and can only handle outside contributions.
Created attachment 83552 [details] Patch for bug in ErrorRecoveryLog.java
For generating unique temp files, the getLocalRandomIdentifier() adds a time stamp to a given file name and then verifies that the new file doesn't yet exist in the file system. The bug is that the path of the file was forgotten for the test, so the new file seems always unique even if it is not. I have attached a patch for org.eclipse.update.internal.core.ErrorRecoveryLog. Best regards, Stephan
Can you please include my patch into Eclipse 3.3.2? Is there anything missing, where I could help?
Is there a reason that the bug fix is not included yet?
Created attachment 88805 [details] JUnit test case This patch demonstrates the bug (and confirms the patch). Add this to org.eclipse.update.tests.core
This should be fixed.
I started to modify the patch to handle the case where #getParentFile returns null and I made the mistake of trying to understand the code. It looks like the code (even before the patch) will return either the path to a file, a directory, or null. It doesn't look like any of the callers handle the null case and I'm not even sure the directory case is handled. Dejan, do you remember what this code is doing?
(In reply to comment #9) I *guess* it is creating a unique file name to generate a file and then make a swap by renaming the temp. file with the actual target name. I remember it was done to ensure that the file writing phase does not crash half way leaving a corrupted file.
Andrew, I suggest just releasing the provided patch as-is. There is a theoretical NPE if someone passes the file system root ("/" or "c:\") to this method, but that was the case before the patch as well. It's not clear there is a reasonable alternative result to return in this case, and from looking at the callers it never happens anyway.
done in HEAD & 3.4.1 (patch + test)
