-
Notifications
You must be signed in to change notification settings - Fork 283
Description
It looks like there are problems using Jimfs in an environment where user and system code are loaded by separate classloaders. In that case, JimfsFileSystemProvider
will be loaded and initialized by the system classloader (because of the META-INF/services
entry), while a user who wants to create a file system by calling Jimfs.newFileSystem
will be using classes loaded by a different classloader.
The problem is that to create a FileSystem
instance, Jimfs
calls FileSystems.newFileSystem(...)
, passing it an environment map containing the Configuration
object to be used to configure the file system. When the JimfsFileSystemProvider
instance loaded by the system classloader gets this object, it doesn't recognize it as an instance of Configuration
, because it was loaded by a different classloader. This leads to an error like:
env map ({config=com.google.common.jimfs.Configuration@15b71125}) must contain key 'config' mapped to an instance of Jimfs.Configuration
Why do we do it this way in the first place? We want to call FileSystems.newFileSystem
so that the canonical (system-loaded) JimfsFileSystemProvider
instance is used to create (and more importantly, cache) the file system. This is necessary for methods like Paths.get(URI)
to work, as it will go to that FileSystemProvider
instance and ask it to resolve the URI
to a Path
. If it doesn't have the FileSystem
instance cached, it won't be able to find it and get a Path
from it.
Some possible solutions (and the problems with them):
- Change the environment map that's passed to
FileSystems.newFileSystem
to only contain standard JDK types. This should solve the classloader issues.- Problem: While most of the
Configuration
object could be converted to a map of standard JDK types, there are a couple things that cannot, specifically thePathType
and the set ofAttributeProvider
s the file system should use. It's possible (though it would require changing the API) that we could change the configuration from storing instances of these types to storing only the classes (requiring a default constructor or some such), in which case we could put the names of the classes into the map to be loaded and instantiated by theFileSystemProvider
. - Even if we did this, though, there are still potential problems if, say, an
AttributeProvider
deals with attributes whose values are not standard JDK classes--the could be problems when a user tries to set an attribute value on a file and theAttributeProvider
doesn't see it as the same class (due to the classloader issues).
- Problem: While most of the
- Stop using
FileSystems.newFileSystem
to create Jimfs file systems. Use a locally initialized instance ofJimfsFileSystemProvider
instead. This solves the problem because only the user code classloader should be involved.- Problem: It's no longer possible to get a Jimfs
Path
orFileSystem
by itsURI
using the standard methods in thejava.nio.file
API. This also prevents things like the recently-added support for JimfsURL
s from working.
- Problem: It's no longer possible to get a Jimfs
- Try to work around this by making the system-loaded
FileSystemProvider
for Jimfs not be the realJimfsFileSystemProvider
.- Rather, it would just be used for caching
FileSystem
instances as needed forURI
-based methods to work. - The real
JimfsFileSystemProvider
would be a singleton, andJimfs
would use that to create aFileSystem
instance whenJimfs.newFileSystem
is called. It would then pass the createdFileSystem
itself as a value in the environment map toFileSystems.newFileSystem
, allowing the newFileSystem
to be cached. SinceFileSystem
is a JDK type and since the system-loadedFileSystemProvider
has no reason to care about any details beyond that, it should have no problem with theFileSystem
coming from a different classloader. - Since the
FileSystemProvider
that created theFileSystem
will be the one that handles implementing all the methods for it, there should be no issues with theFileSystem
functionality.
- Rather, it would just be used for caching
I think approach 3 should work, but still need to confirm that. It's fairly gross, but should be transparent as long as people are using the API as expected.