Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [equinox-dev] Reliable Testing For a Writable Folder
Hi Ed

Have you checked of java.nio is better? Files.isWritable?

If this is just a windows problem having a guard makes sense. 

If just a windows 7 problem perhaps you can just wait 3 weeks and then Eclipse can drop official support for Windows 7 (if multibillion dollar Microsoft isn't going to support Windows 7 after next month, not sure the Eclipse community should spend time on it - but I guess there will be lots of resistance to that idea)

https://support.microsoft.com/en-ca/help/4057281/windows-7-support-will-end-on-january-14-2020

HTH, 
Jonah


On Mon., Dec. 23, 2019, 06:36 Ed Merks, <ed.merks@xxxxxxxxx> wrote:

While investigating https://bugs.eclipse.org/bugs/show_bug.cgi?id=558462 I tracked it down to the fact that testing a folder's writability, modifies the folder's timestamp if the folder is writable.

There is very similar (copied) logic in all the following places:

  org.eclipse.equinox.launcher.Main.canWrite(File)
  org.eclipse.osgi.storage.StorageUtil.canWrite(File)
  org.eclipse.equinox.internal.p2.director.app.DirectorApplication.canWrite(File)
  org.eclipse.equinox.internal.p2.artifact.repository.Activator.isReadOnly(File)

Each takes the approach of creating a file in the folder and then deleting it, changing the folder's timestamp, if the temp file could be created.

The Activator.isReadOnly(File) logic specifically is what causes side-effects in the file system when loading an artifact repository via a file: URI when the running JVM has write access to that folder.  This is  what caused the mirrors for 2019-03 through 2019-12 to be invalidated daily, but anything loading artifact metadata via p2's APIs can result in this same problem.

I considered that perhaps calling File.lastModified and File.setLastModified is a reasonable workaround, e.g., like this:

    private boolean isReadOnly(File file) {
        if (file == null)
            return true; // If we've reached the root, then return true
        if (file.exists()) {
            try {
                // on Vista/Windows 7 you are not allowed to write executable files on virtual directories like "Program Files"
                long lastModified = file.lastModified();
                File tmpTestFile = File.createTempFile(".artifactlocktest", ".dll", file); //$NON-NLS-1$ //$NON-NLS-2$
                tmpTestFile.delete();
                file.setLastModified(lastModified);
                return false;
            } catch (IOException e) {
                return true; // permission issue to create new file, so it's readonly
            }
        }

        return isReadOnly(file.getParentFile());
    }

This does restore the lastModified value, except for the very short time interval while creating the temp file and deleting it...

Perhaps there's a better way to test for writability if we must continue to assume that java.io.File.canWrite() is unreliable.

It seems all this is there only for Vista, though for Activator.isReadOnly, the comment mentions Windows 7:

  // on Vista/Windows 7 you are not allowed to write executable files on virtual directories like "Program Files"

So I tested on my Windows 7 machine and indeed when I create the folder C:/Program Files/Test, it's not possible to create a temp file in that folder (regardless of extension) even though File.canWrite returns true.

Does anyone have any thoughts on how we might improve this behavior?  Note that I'm not sure whether or not the other 3 implementations might also cause similar unfortunate side-effects...

Perhaps they should all be guarded to apply only on Windows?

Regards,
Ed








_______________________________________________
equinox-dev mailing list
equinox-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://www.eclipse.org/mailman/listinfo/equinox-dev

Back to the top