Skip to content

Commit

Permalink
re #836: VFSClassLoader multi-thread resource loading
Browse files Browse the repository at this point in the history 
* Updated the class loader tests to show the errors
* Updated ZipFileSystem and TarFileSystem with threadlocal files
  • Loading branch information
@ivakegg
ivakegg committed May 7, 2023
1 parent 2e1fd69 commit 46dbafb
Show file tree
Hide file tree
Showing 7 changed files with 289 additions and 75 deletions.
2 changes: 1 addition & 1 deletion commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpClientFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ private void configureClient(final FileSystemOptions fileSystemOptions, final C
* @return A new connection.
* @throws FileSystemException if an error occurs while connecting.
*/
public C createConnection(final String hostname, final int port, char[] username, char[] password,
public synchronized C createConnection(final String hostname, final int port, char[] username, char[] password,
final String workingDirectory, final FileSystemOptions fileSystemOptions) throws FileSystemException {
// Determine the username and password to use
if (username == null) {
Expand Down
92 changes: 65 additions & 27 deletions commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/tar/TarFileSystem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,6 @@
*/
package org.apache.commons.vfs2.provider.tar;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.zip.GZIPInputStream;

import org.apache.commons.compress.archivers.ArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
Expand All @@ -44,6 +34,16 @@
import org.apache.commons.vfs2.provider.UriParser;
import org.apache.commons.vfs2.provider.bzip2.Bzip2FileObject;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.zip.GZIPInputStream;

/**
* A read-only file system for Tar files.
*/
Expand All @@ -54,7 +54,53 @@ public class TarFileSystem extends AbstractFileSystem {

private final File file;

private TarArchiveInputStream tarFile;
private TarFileThreadLocal tarFile = new TarFileThreadLocal();

private class TarFileThreadLocal {

private ThreadLocal<Boolean> isPresent = new ThreadLocal<Boolean>() {
@Override
protected Boolean initialValue() {
return Boolean.FALSE;
}
};
private ThreadLocal<TarArchiveInputStream> tarFile = new ThreadLocal<TarArchiveInputStream>() {
@Override
public TarArchiveInputStream initialValue() {
if (isPresent.get()) {
throw new IllegalStateException("Creating an initial value but we already have one");
}
try {
isPresent.set(Boolean.TRUE);
return createTarFile(TarFileSystem.this.file);
} catch (FileSystemException fse) {
throw new RuntimeException(fse);
}
}
};

public TarArchiveInputStream getFile() throws FileSystemException {
try {
return tarFile.get();
} catch (RuntimeException e) {
if (e.getCause() instanceof FileSystemException) {
throw new FileSystemException(e.getCause());
}
else {
throw new RuntimeException(e);
}
}
}

public void closeFile() throws IOException {
if (isPresent.get()) {
TarArchiveInputStream file = tarFile.get();
file.close();
tarFile.remove();
isPresent.set(Boolean.FALSE);
}
}
}

/**
* Cache doesn't need to be synchronized since it is read-only.
Expand Down Expand Up @@ -117,10 +163,7 @@ protected TarFileObject createTarFileObject(final AbstractFileName name, final T
protected void doCloseCommunicationLink() {
// Release the tar file
try {
if (tarFile != null) {
tarFile.close();
tarFile = null;
}
tarFile.closeFile();
} catch (final IOException e) {
// getLogger().warn("vfs.provider.tar/close-tar-file.error :" + file, e);
VfsLog.warn(getLogger(), LOG, "vfs.provider.tar/close-tar-file.error :" + file, e);
Expand All @@ -147,6 +190,7 @@ public InputStream getInputStream(final TarArchiveEntry entry) throws FileSystem
resetTarFile();
try {
ArchiveEntry next;
TarArchiveInputStream tarFile = getTarFile();
while ((next = tarFile.getNextEntry()) != null) {
if (next.equals(entry)) {
return tarFile;
Expand All @@ -159,10 +203,7 @@ public InputStream getInputStream(final TarArchiveEntry entry) throws FileSystem
}

protected TarArchiveInputStream getTarFile() throws FileSystemException {
if (tarFile == null && this.file.exists()) {
recreateTarFile();
}
return tarFile;
return tarFile.getFile();
}

@Override
Expand Down Expand Up @@ -225,15 +266,12 @@ protected void putFileToCache(final FileObject file) {
*/

private void recreateTarFile() throws FileSystemException {
if (this.tarFile != null) {
try {
this.tarFile.close();
} catch (final IOException e) {
throw new FileSystemException("vfs.provider.tar/close-tar-file.error", file, e);
}
tarFile = null;
try {
tarFile.closeFile();
} catch (final IOException e) {
throw new FileSystemException("vfs.provider.tar/close-tar-file.error", file, e);
}
this.tarFile = createTarFile(this.file);
tarFile.getFile();
}

/**
Expand Down
87 changes: 60 additions & 27 deletions commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/zip/ZipFileSystem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,6 @@
*/
package org.apache.commons.vfs2.provider.zip;

import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
import java.util.Collection;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Map;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.commons.vfs2.Capability;
Expand All @@ -39,6 +29,16 @@
import org.apache.commons.vfs2.provider.AbstractFileSystem;
import org.apache.commons.vfs2.provider.UriParser;

import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
import java.util.Collection;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Map;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;

/**
* A read-only file system for ZIP and JAR files.
*/
Expand All @@ -49,7 +49,53 @@ public class ZipFileSystem extends AbstractFileSystem {

private final File file;
private final Charset charset;
private ZipFile zipFile;
private ZipFileThreadLocal zipFile = new ZipFileThreadLocal();

private class ZipFileThreadLocal {

private ThreadLocal<Boolean> isPresent = new ThreadLocal<Boolean>() {
@Override
protected Boolean initialValue() {
return Boolean.FALSE;
}
};
private ThreadLocal<ZipFile> zipFile = new ThreadLocal<ZipFile>() {
@Override
public ZipFile initialValue() {
if (isPresent.get()) {
throw new IllegalStateException("Creating an initial value but we already have one");
}
try {
isPresent.set(Boolean.TRUE);
return createZipFile(ZipFileSystem.this.file);
} catch (FileSystemException fse) {
throw new RuntimeException(fse);
}
}
};

public ZipFile getFile() throws FileSystemException {
try {
return zipFile.get();
} catch (RuntimeException e) {
if (e.getCause() instanceof FileSystemException) {
throw new FileSystemException(e.getCause());
}
else {
throw new RuntimeException(e);
}
}
}

public void closeFile() throws IOException {
if (isPresent.get()) {
ZipFile file = zipFile.get();
file.close();
zipFile.remove();
isPresent.set(Boolean.FALSE);
}
}
}

/**
* Cache doesn't need to be synchronized since it is read-only.
Expand All @@ -71,12 +117,6 @@ public ZipFileSystem(final AbstractFileName rootFileName, final FileObject paren
// Make a local copy of the file
file = parentLayer.getFileSystem().replicateFile(parentLayer, Selectors.SELECT_SELF);
this.charset = ZipFileSystemConfigBuilder.getInstance().getCharset(fileSystemOptions);

// Open the Zip file
if (!file.exists()) {
// Don't need to do anything
zipFile = null;
}
}

/**
Expand Down Expand Up @@ -111,12 +151,9 @@ protected ZipFileObject createZipFileObject(final AbstractFileName name, final Z

@Override
protected void doCloseCommunicationLink() {
// Release the zip file
// Release the zip files
try {
if (zipFile != null) {
zipFile.close();
zipFile = null;
}
zipFile.closeFile();
} catch (final IOException e) {
// getLogger().warn("vfs.provider.zip/close-zip-file.error :" + file, e);
VfsLog.warn(getLogger(), LOG, "vfs.provider.zip/close-zip-file.error :" + file, e);
Expand All @@ -136,11 +173,7 @@ protected FileObject getFileFromCache(final FileName name) {
}

protected ZipFile getZipFile() throws FileSystemException {
if (zipFile == null && this.file.exists()) {
this.zipFile = createZipFile(this.file);
}

return zipFile;
return zipFile.getFile();
}

@Override
Expand Down
Loading

0 comments on commit 46dbafb

Please sign in to comment.