Community
Participate
Working Groups
Consider the following scenario: A project, which is located on a local disk, contains a linked folder that maps to a directory on NFS. Eclipse has few editors open on resources corresponding to NFS files. User leaves Eclipse up for the weekend. Eclipse looses access to NFS due to expiration of the Kerberos ticket. When Eclipse refreshes its resources by periodic polling, it thinks that the files and directories on NFS have been deleted. Eclipse removes the corresponding resources and closes all editors associated with the removed resources. When the user comes to work on Monday and renews the Kerberos ticket, the state of open editors that Eclipse had on Friday is lost. CDT projects suffer even more since they react to the resource delta by removing the corresponding files from the index. Restoring the original state of the index is a lengthy operation. The root cause of the problem is that org.eclipse.core.internal.filesystem.local.unix.UnixFileNatives.fetchFileInfo(String) does not distinguish between nonexistent files and I/O errors. Furthermore, the current IFileStore/IFileInfo API doesn't allow for such distinction.
Created attachment 215514 [details] Proposed fix Could please somebody review the attached proposed fix. The patch is intended for post-Juno and doesn't yet include changes to the native Windows code. If you agree with the general approach, I can add the missing Windows part.
Created attachment 215544 [details] Proposed fix Fixed an error that was introduced when creating the previous patch.
Did you explore the idea that exists() should return true in the case of an error? Can you check what java.io.File#exists does in this case? What about a bash script test like if [ -e $file ]. Generally being consistent with native tools on that platform is the best answer.
(In reply to comment #3) > Did you explore the idea that exists() should return true in the case of an > error? This would be a deviation from java.io.File semantics (see below). > Can you check what java.io.File#exists does in this case? java.io.File returns false in case of errors although the Javadoc doesn't explicitly say that. in Java 1.7 java.nio.file.Files.exists(Path, LinkOption...) returns {@code true} if the file exists; {@code false} if the file does not exist or its existence cannot be determined. java.nio.file.Files.notExists(Path, LinkOption...) returns {@code true} if the file does not exist; {@code false} if the file exists or its existence cannot be determined > What about a > bash script test like if [ -e $file ]. Generally being consistent with native > tools on that platform is the best answer. Agree. -e $file also returns false in case of error.
John, could you please give your blessing to the general approach taken in the patch. I would then add the native Windows code and push the updated patch to Gerrit.
Created attachment 218138 [details] Proposed fix This is a complete set of changes including native code for Windows and a prebuilt Win32 localfile_1_0_0.dll. Two remaining dlls for other Windows architectures have to be built by somebody who has access to the appropriate machines. John, I'll appreciate if you can review the patch. Thanks.
Hi Sergey, could you push the fix to https://git.eclipse.org/r/p/platform/eclipse.platform.resources.git, please? Thanks
(In reply to comment #7) > Hi Sergey, could you push the fix to > https://git.eclipse.org/r/p/platform/eclipse.platform.resources.git, please? > Thanks Done: https://git.eclipse.org/r/6599
(In reply to comment #8) > (In reply to comment #7) > > Hi Sergey, could you push the fix to > > https://git.eclipse.org/r/p/platform/eclipse.platform.resources.git, please? > > Thanks > > Done: https://git.eclipse.org/r/6599 The patch looks good in general. You need to update version in pom.xml to 1.4.0. It would be also nice to have test cases for the new thing.
I like the idea. One suggest is to change the boolean hasError to something with an int return value (maybe getError). For now it could just return something generic like IO_ERROR or NONE, but this would allow us in the future to expand this to expose different kinds of errors (like errno in the unix stat function).
(In reply to comment #10) > I like the idea. One suggest is to change the boolean hasError to something > with an int return value (maybe getError). For now it could just return > something generic like IO_ERROR or NONE, but this would allow us in the > future to expand this to expose different kinds of errors (like errno in the > unix stat function). Feel free to open a separate bug for tests, we can release them in a separate commit. When you do the change to pom.xml and to hasError (as per John's suggestion), please push the new fix to gerrit again.
(In reply to comment #11) An updated patch has been pushed to Gerrit. This patch includes pom.xml file that I missed last time (my apology for that) and IFileInfo.getError method as John suggested. I'd be glad to add tests but could quite figure out how to simulate file errors. Any ideas for how to do that will be appreciated. I've created bug 389337 to track test creation.
This fix looks good to me. Two small things: - FileInfo.setError still takes a boolean, even though getError returns an int. Seems like we should make this consistent. - Can you provide compiled win64 dll as well?
(In reply to comment #13) I'm on currently on vacation and will make the requested changes when I come back in mid October. I won't be able to compile for win64 since I don't have access to such machine.
(In reply to comment #13) > This fix looks good to me. Two small things: > > - FileInfo.setError still takes a boolean, even though getError returns an > int. Seems like we should make this consistent. > - Can you provide compiled win64 dll as well? This fix is not good. On Windows 7 32-bit I now get a log entry each time I start (see below). Yes, I know, the entry says something about "may not be an error", but how can I know? Why log it then in the first place? This log entry has to go away again and only be logged if it is really missing on a platform where it is expected to exist. !ENTRY org.eclipse.core.filesystem 1 1 2012-10-05 10:48:20.646 !MESSAGE Could not load library: localfile_1_0_0.dll. This library provides platform-specific optimizations for certain file system operations. This library is not present on all platforms, so this may not be an error. The resources plug-in will safely fall back to using java.io.File functionality. !STACK 0 java.lang.UnsatisfiedLinkError: C:\eclipse\drops\4.3-N20121004-2000\configuration\org.eclipse.osgi\bundles\40\1\.cp\os\win32\x86\localfile_1_0_0.dll: Can't load this .dll (machine code=0xbd) on a IA 32-bit platform
John, it would be good to mention in the bug report when a fix gets pushed to the shared repository. When I looked at http://git.eclipse.org/c/platform/eclipse.platform.resources.git/ I was first quite confused to see a 9 day old fix not being part of this weeks I-build. I assume you committed it locally and then for some reason decided to push it only one week later.
(In reply to comment #15) > (In reply to comment #13) > > This fix looks good to me. Two small things: > > > > - FileInfo.setError still takes a boolean, even though getError returns an > > int. Seems like we should make this consistent. > > - Can you provide compiled win64 dll as well? > > This fix is not good. On Windows 7 32-bit I now get a log entry each time I... The new dll does not load and we lost functionality on win32.x86. Sergey was asked to provide win.x64 dll, so I would expect he will fix x32 dll in the same time. He is back in mid Oct. Till then, I think we can rollback to the previous version of dll.
I never intended to push this. I cherry-picked Sergey's patch last week in order to test it. It wasn't ready yet, so I gave some comments here and left it. I should have reverted my local master but I forgot. Yesterday I made a completely unrelated change and pushed, and missed that this cherry-picked commit was still in my local master. I messed up here and I have reverted it: http://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/?id=b4719f7c958d562ea2138c6493a6a7e057194507
We are mostly done with M3, moving to M4.
(In reply to comment #13) Uploaded an updated patch to https://git.eclipse.org/r/#/c/6599/. Please review.
This latest patch looks good to me. We just need to track down a win64 machine with compiler license to create a binary there. The org.eclipse.core.filesystem bundle version needs a minor increment for the new API. No need to submit a new patchset for that Sergey, we can just make the change locally in manifest and pom.
The latest patch is merged to master. I built the lib for win32.x86_64 and will build for ia64 tomorrow. Then I will close the bug. Thanks Sergey.
Thanks a lot!
Should this bug be marked as fixed?
Yes this was in M4.
