Skip to content

Created a better thread safety test and fixed a couple thread safety issues #395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ivakegg wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Created a better thread safety test and fixed a couple thread safety issues #395

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
db94749
re #836: VFSClassLoader multi-thread resource loading
ivakegg May 3, 2023
ace31a8
re #836: Threaded VFS safety
ivakegg May 10, 2023
f7899db
re #836: Removed redundant modifiers
ivakegg May 10, 2023
8e5be25
re #836: Reverted to explicitly include new test cases instead of
ivakegg May 10, 2023
bb4f8af
re #836: removed unused imports
ivakegg May 10, 2023
e152b17
re #836: removed thread safety test for resource provider
ivakegg May 10, 2023
a125800
re #836: inadvertant change reverted
ivakegg May 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpClientFactory.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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