ClassLoader issues with Jimfs.newFileSystem #18
Comments
|
Created a commit implementing approach 3: https://github.com/cgdecker/jimfs/commit/029a05e314ceeb9da7688cb3060c48c986343e03 All tests pass, but I need to try it out on App Engine or something else that would exhibit the class loading problems. |
|
The only thing I'm wondering is whether we can make the system-loaded Regardless, your reasoning behind approach 3 sound good to me. |
I have no idea. I guess that since the system class loader loads the I've verified that approach 3 works and created a test for it. It's slightly weird but in general not too bad in my opinion. The grossest thing about it is is that it requires a couple of reflective method calls at places where the system-loaded |
|
SGTM. Oh, I meant to also make the obvious observation that someone on Stack Overflow might know the answer. Then I meant to make the equally obvious observation that probably that won't happen. But it might be worth a shot. Up to you. |
See #18 for more information. - SystemJimfsFileSystemProvider (which is loaded as a service by the system classloader) only serves as a cache of FileSystem instances as needed to make Paths.get(URI) work. - JimfsFileSystemProvider is the actual provider that is returned by JimfsFileSystem.provider(). It is a singleton and provides the real implementation of most of the FileSystemProvider methods, but does not cache FileSystem instances. We now create the FileSystem instances directly when the user calls Jimfs.newFileSystem rather than passing the Configuration off to FileSystems.newFileSystem. This should ensure no classloader issues when creating the FileSystem. Once created, the FileSystem itself is passed to FileSystems.newFileSystem. There should be no classloader issues here either, since the new SystemJimfsFileSystemProvider implementation does not deal with any other Jimfs-specific classes. Also add a test that fails without this change. ------------- Created by MOE: http://code.google.com/p/moe-java MOE_MIGRATED_REVID=87170296
|
Fixed in 9acd047. |
|
is there a place where I can reference a release (in pom) that contains this fix? thanks. |
|
Yes, Jimfs 1.1 contains this fix. |
|
I see. Just to make sure. The 1.1 release is on 2/12 (http://mvnrepository.com/artifact/com.google.jimfs/jimfs/1.1), but the check in seems to be on 2/25: that’s why I suspect it’s not in. I am having problem with the following statement: FileSystem fs = Jimfs.newFileSystem(Configuration.unix()); Running in eclipse. It gives me this exception: java.lang.NoSuchMethodError: com.google.common.collect.ImmutableSet.copyOf(Ljava/util/Collection;)Lcom/google/common/collect/ImmutableSet; sm From: Colin Decker <notifications@gh.neting.ccmailto:notifications@github.com> Yes, Jimfs 1.1 contains this fix. — |
|
The 1.1 release was on 2/12/2016, while the change was made in 2015. The error you're seeing likely means you have a very old version of Guava (or perhaps even google-collections, its predecessor) on your classpath (perhaps transitively through some other dependency). You'll need to find out where that's coming from ( |
|
Thanks. you’re right. it’s due to the old version of the google collection. Replacing the lib with guava and the issue is gone. sm From: Colin Decker <notifications@gh.neting.ccmailto:notifications@github.com> The 1.1 release was on 2/12/2016, while the change was made in 2015. The error you're seeing likely means you have a very old version of Guava (or perhaps even google-collections, its predecessor) on your classpath (perhaps transitively through some other dependency). You'll need to find out where that's coming from (mvn dependency:tree might help) and deal with it somehow... ideally by upgrading to a new version of that library that depends on a newer version of Guava, or else by excluding Guava/google-collections from that library's dependencies. — |
…priately. Summary: The Thrift API for Eden fails if the path passed to `getSHA1()` is a symlink. This adds logic to Buck to try to recover from this in a graceful way. This upgrades us from jimfs-1.0 to jimfs-1.1 to pick up this fix: google/jimfs#18 Test Plan: Introduced a new unit test: `EdenProjectFilesystemDelegateTest`. Incidentally, this is our first use of PowerMock in Buck. Reviewed By: yiding fbshipit-source-id: a472b95
It looks like there are problems using Jimfs in an environment where user and system code are loaded by separate classloaders. In that case,
JimfsFileSystemProviderwill be loaded and initialized by the system classloader (because of theMETA-INF/servicesentry), while a user who wants to create a file system by callingJimfs.newFileSystemwill be using classes loaded by a different classloader.The problem is that to create a
FileSysteminstance,JimfscallsFileSystems.newFileSystem(...), passing it an environment map containing theConfigurationobject to be used to configure the file system. When theJimfsFileSystemProviderinstance loaded by the system classloader gets this object, it doesn't recognize it as an instance ofConfiguration, because it was loaded by a different classloader. This leads to an error like:Why do we do it this way in the first place? We want to call
FileSystems.newFileSystemso that the canonical (system-loaded)JimfsFileSystemProviderinstance is used to create (and more importantly, cache) the file system. This is necessary for methods likePaths.get(URI)to work, as it will go to thatFileSystemProviderinstance and ask it to resolve theURIto aPath. If it doesn't have theFileSysteminstance cached, it won't be able to find it and get aPathfrom it.Some possible solutions (and the problems with them):
FileSystems.newFileSystemto only contain standard JDK types. This should solve the classloader issues.Configurationobject could be converted to a map of standard JDK types, there are a couple things that cannot, specifically thePathTypeand the set ofAttributeProviders 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.AttributeProviderdeals 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 theAttributeProviderdoesn't see it as the same class (due to the classloader issues).FileSystems.newFileSystemto create Jimfs file systems. Use a locally initialized instance ofJimfsFileSystemProviderinstead. This solves the problem because only the user code classloader should be involved.PathorFileSystemby itsURIusing the standard methods in thejava.nio.fileAPI. This also prevents things like the recently-added support for JimfsURLs from working.FileSystemProviderfor Jimfs not be the realJimfsFileSystemProvider.FileSysteminstances as needed forURI-based methods to work.JimfsFileSystemProviderwould be a singleton, andJimfswould use that to create aFileSysteminstance whenJimfs.newFileSystemis called. It would then pass the createdFileSystemitself as a value in the environment map toFileSystems.newFileSystem, allowing the newFileSystemto be cached. SinceFileSystemis a JDK type and since the system-loadedFileSystemProviderhas no reason to care about any details beyond that, it should have no problem with theFileSystemcoming from a different classloader.FileSystemProviderthat created theFileSystemwill be the one that handles implementing all the methods for it, there should be no issues with theFileSystemfunctionality.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.
The text was updated successfully, but these errors were encountered: