JDK-8213406 : (fs) More than one instance of built-in FileSystem observed in heap
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.nio
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2018-11-06
  • Updated: 2019-09-12
  • Resolved: 2018-11-28
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 11 JDK 12
11.0.5-oracleFixed 12 b23Fixed
Related Reports
Relates :  
Description
We have a local mod that needed access to the builtin FileSystem, and used 
sun.nio.fs.DefaultFileSystemProvider.create()
as is done in FileSystems.java and FilePermissions.java
and we ran into

Caused by: java.lang.UnsupportedOperationException: Path not associated with default file system.
        at java.base/java.nio.file.Path.toFile(Path.java:771)

and the root cause was that we had two instances of LinuxFileSystem, but much of nio code assumes there's exactly one instance of the default FileSystem.  Here's a demo that stock jdk12 can end up with more than one LinuxFileSystem instance:

If you run this program
----------
public class Sleeper {
    public static void main(String[] args) throws Throwable {
        java.nio.file.FileSystems.getDefault();
        Runtime.getRuntime().exec(new String[] {"/bin/sleep", "120"}).waitFor();
    }
}
-----------
and then  $ jmap -histo 213201 | grep LinuxFileSystem
 216:             2             64  sun.nio.fs.LinuxFileSystem (java.base@12-ea)
 278:             2             32  sun.nio.fs.LinuxFileSystemProvider (java.base@12-ea)

I took a look at FilePermission to see if I could identify a user-visible bug as a result of the duplicate instances, but came up empty.  Nevertheless, it is probably intended to have only one - that helps with maintainer sanity and keep down startup overhead.

---

It looks like the intent is to have only a single instance, but there is no easy way for code inside java.base to access the one true builtin FileSystemProvider.  
Here's an experimental change that works for us:

@@ -34,10 +34,14 @@
 public class DefaultFileSystemProvider {
     private DefaultFileSystemProvider() { }
 
+    private static final LinuxFileSystemProvider theFileSystemProvider
+        = new LinuxFileSystemProvider();
+
     /**
      * Returns the default FileSystemProvider.
      */
     public static FileSystemProvider create() {
-        return new LinuxFileSystemProvider();
+        return theFileSystemProvider;
     }
 }

but of course it's incomplete...

The name "create" strongly suggests a new instance is returned, but those are not the sermantics we want.  We could rename it to "get" or "instance".
Of course we would want to fix all the OSes, not just Linux.  Maybe there's a refactoring that works well.  Maybe we should migrate those Holder classes from FileSystems.java into a location where it can be used by the rest of java.base.
Comments
Fix request (11u) Requesting this backport for parity with Oracle 11 Updates. Patch needs to be pushed to 11u because it's part of Oracle 11.0.5. Patch applies cleanly. Will be tested in SAP's test landscape before pushing.
03-09-2019

This change enabled resolving FilePermissions lazily: by ensuring that the file system objects are singletons we can be sure the sole instance can be initialized before installing a security manager, which avoids recursively dropping into FilePermission checks while initializing FilePermission itself.
20-08-2019

After some offline discussion, Alan and I converged on a change we both find acceptable. https://cr.openjdk.java.net/~martin/webrevs/jdk/unique-builtin-filesystem/ with more backups in the usual location.
27-11-2018

> Preference wise I'd prefer to keep theFileSystem provider and add an accessor method I guess you're looking to get back a method in DefaultFileSystemProvider that returns a FileSystemProvider. But we don't want to call it create(). What would you like? Perhaps get() ?
12-11-2018

I can certainly drop the spec change for FileSystemProvider, but ... are you actually saying that it's a requirement on third party FileSystemProvider implementations that they must create their unique filesystem eagerly if they are being used as the default (not system-default) provider? (I don't understand - why are we even specifying the degree of laziness?) * Where * a provider is the factory for a single file system then it is provider dependent * if the file system is created when the provider is initialized, or later when - * the {@code newFileSystem} method is invoked. In the case of the default + * the {@code newFileSystem} method is invoked. In the case of the system-default * provider, the {@code FileSystem} is created when the provider is initialized.
12-11-2018

I don't think we should change the FileSystemProvider spec as that is read by FileSystemProvider implementers. Preference wise I'd prefer to keep theFileSystem provider and add an accessor method but this is a minor point compared to the spec change.
12-11-2018

I added the other platforms to the webrev. This change is now complete except for review and cross-platform testing.
12-11-2018

TIL about "system-default provider". i tried to use that wording in the latest iteration. I noticed that the system-default providers all eagerly create a FileSystem. We can save a bit of startup overhead by making theFileSystem available in the package and not pointlessly checking the URI. https://cr.openjdk.java.net/~martin/webrevs/jdk/unique-builtin-filesystem/ backups: https://cr.openjdk.java.net/~martin/webrevs/jdk/unique-builtin-filesystem.0/ https://cr.openjdk.java.net/~martin/webrevs/jdk/unique-builtin-filesystem.1/
09-11-2018

The terminology in the specification is "default provider" vs. "system-default provider". It should be rare to deploy with java.nio.file.spi.DefaultFileSystemProvider set so most developers will not need to be concerned with this detail. The update webrev looks okay to me, assuming all platform versions of DefaultFileSystemProvider.java are updated.
09-11-2018

Alan, I modified the change as you suggested https://cr.openjdk.java.net/~martin/webrevs/jdk/unique-builtin-filesystem/ but I (naturally) prefer my original https://cr.openjdk.java.net/~martin/webrevs/jdk/unique-builtin-filesystem.00/ because of the confusion about "default". In particular, I now have public static FileSystem getDefault() { if (VM.isModuleSystemInited()) { return DefaultFileSystemHolder.defaultFileSystem; } else { - return BuiltinFileSystemHolder.builtinFileSystem; + return DefaultFileSystemProvider.FILE_SYSTEM; } where the word "Default" is now used in two slightly different ways - "true default" and "default default". But both variants of this patch seem OK to me - you choose! (Another possibly better name for the "built-in" one is PlatformFileSystemProvider)
09-11-2018

The original intention (and implementation) was that it would be a singleton and DefaultFileSystemProvider.create would be called only once. Startup issues complicated this a bit in that the platform default file system provider needs to be used early in the startup. The re-implementation of FilePermission has also introduced complications. The approach in the proposed patch is okay but I would prefer to keep DefaultFileSystemProvider and name the fields fields FILE_SYSTEM_PROVIDER and FILE_SYSTEM so that the "built-in" term goes away or is at least limited to simple comment in FileSystems.getDefault for the early startup case.
09-11-2018

I agree that from current openjdk's point of view this can be considered cleanup, and a small startup+footprint optimization. Future maintainers may assume the existence of a single instance. For Google, the future is today - we ran into the UOE because we created third instances of these "singleton" objects.
08-11-2018

VM initialization changed very significantly in JDK 9, the multi phase initialization and other changes in JDK 9 means we can use the new file system API in initPhase2 whereas previously we had several recursive initialization issues when attempting to use it early in the startup. Some of the startup optimizations in JDK 9/10/11 have changed the point where it is first used. The FilePermission work in JDK 9 also introduced several complications and it does not surprise me that some clean-up is required (which is really what this issue is about). I've made a note to look at the webrev.
08-11-2018

OK, here's a serious change for you to consider https://cr.openjdk.java.net/~martin/webrevs/jdk/unique-builtin-filesystem/ I tried to consistently use "builtin" instead of "default" since the latter is ambiguous in this context. FileSystems has one fewer Holder classes. Was there a reason to delay the creation of the builtin filesystem after the creation of the builtin filesystem provider? FilePermission now uses the same FileSystem instance as the rest of the JDK. We can tighten access by making LinuxFileSystemProvider package-private. The only thing missing is extending the pattern from linux to the other OSes that have provider implementations. I'll obviously do that before commit.
08-11-2018

Sorry, I remember several times I see exceptions thrown during the development of FilePermission, but I'm not sure what the ultimate reason is to use the current call. Maybe there is "a custom default provider" in one test, and maybe I see "recursive initializations" or "FileSystems.getDefault()" is not ready when the first FilePermission is created.
06-11-2018

> There are potential recursive initialization and bootstrapping issues here. Probably we use the builtin file system for the same reason FilePermission does - to do "real" file system operations before the higher level file system machinery is initialized.
06-11-2018

>> Note that `java.nio.file.spi.DefaultFileSystemProvider` is a system property, not a class: Ohh! It *looks* like a class name. I'll amend the description.
06-11-2018

> * Always use the internal default file system, in case it was modified > * with java.nio.file.spi.DefaultFileSystemProvider. > > which must be wrong since there is no such class in that package. Note that `java.nio.file.spi.DefaultFileSystemProvider` is a system property, not a class: http://hg.openjdk.java.net/jdk/jdk/file/3021c1ad958b/src/java.base/share/classes/java/nio/file/FileSystems.java#l123
06-11-2018

We should have agreement on direction before sorking on a complete changeset.
06-11-2018

There are potential recursive initialization and bootstrapping issues here. I've added Max to the interest list so he can comment on the changes to FilePermission in JDK 9. I believe it uses its instance of the built-in file system provider to avoid using a custom default provider and also to avoid recursive initializations that would arise if it uses FileSystems.getDefault.
06-11-2018