diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpClientFactory.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpClientFactory.java index c5bf58261c..a19af974ca 100644 --- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpClientFactory.java +++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpClientFactory.java @@ -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) { diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/tar/TarFileSystem.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/tar/TarFileSystem.java index df472d49e3..bb47584405 100644 --- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/tar/TarFileSystem.java +++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/tar/TarFileSystem.java @@ -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; @@ -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. */ @@ -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 isPresent = new ThreadLocal() { + @Override + protected Boolean initialValue() { + return Boolean.FALSE; + } + }; + private ThreadLocal tarFile = new ThreadLocal() { + @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. @@ -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); @@ -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; @@ -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 @@ -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(); } /** diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/zip/ZipFileSystem.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/zip/ZipFileSystem.java index 78a57ae373..05b8082b41 100644 --- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/zip/ZipFileSystem.java +++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/zip/ZipFileSystem.java @@ -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; @@ -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. */ @@ -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 isPresent = new ThreadLocal() { + @Override + protected Boolean initialValue() { + return Boolean.FALSE; + } + }; + private ThreadLocal zipFile = new ThreadLocal() { + @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. @@ -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; - } } /** @@ -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); @@ -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 diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/impl/VfsClassLoaderTests.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/impl/VfsClassLoaderTests.java index 533ba6763f..cd13f5ebfd 100644 --- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/impl/VfsClassLoaderTests.java +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/impl/VfsClassLoaderTests.java @@ -16,39 +16,71 @@ */ package org.apache.commons.vfs2.impl; -import static org.apache.commons.vfs2.VfsTestUtils.getTestDirectoryFile; +import org.apache.commons.io.output.StringBuilderWriter; +import org.apache.commons.vfs2.AbstractProviderTestCase; +import org.apache.commons.vfs2.Capability; +import org.apache.commons.vfs2.FileObject; +import org.apache.commons.vfs2.FileSystemException; +import org.apache.commons.vfs2.FileSystemManager; +import org.apache.commons.vfs2.FileType; +import org.apache.commons.vfs2.operations.FileOperationProvider; +import org.junit.Test; import java.io.File; +import java.io.IOException; +import java.io.InputStream; import java.io.PrintWriter; import java.net.URL; import java.net.URLConnection; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Enumeration; import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.Queue; +import java.util.Set; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; +import java.util.concurrent.BrokenBarrierException; +import java.util.concurrent.CyclicBarrier; import java.util.concurrent.RejectedExecutionHandler; import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; -import org.apache.commons.io.output.StringBuilderWriter; -import org.apache.commons.vfs2.AbstractProviderTestCase; -import org.apache.commons.vfs2.Capability; -import org.apache.commons.vfs2.FileObject; -import org.apache.commons.vfs2.FileSystemException; -import org.apache.commons.vfs2.FileSystemManager; -import org.apache.commons.vfs2.FileType; -import org.junit.Test; +import static org.apache.commons.vfs2.VfsTestUtils.getTestDirectoryFile; /** * VfsClassLoader test cases. */ public class VfsClassLoaderTests extends AbstractProviderTestCase { + final static Map TEST_FILES = Arrays.asList(new Object[][] { + {"read-tests/empty.txt",0L}, + {"read-tests/file1.txt",20L}, + //{"read-tests/file%25.txt",20L}, + {"read-tests/dir1/file1.txt",12L}, + {"read-tests/dir1/file2.txt",12L}, + {"read-tests/dir1/subdir2/file1.txt",12L}, + {"read-tests/dir1/subdir2/file2.txt",12L}, + {"read-tests/dir1/subdir2/file3.txt",12L}, + {"read-tests/dir1/subdir3/file1.txt",12L}, + {"read-tests/dir1/subdir3/file2.txt",12L}, + {"read-tests/dir1/subdir3/file3.txt",12L}, + {"read-tests/dir1/file3.txt",12L}, + {"read-tests/dir1/subdir4.jar/file1.txt",12L}, + {"read-tests/dir1/subdir4.jar/file2.txt",12L}, + {"read-tests/dir1/subdir4.jar/file3.txt",12L}, + {"read-tests/dir1/subdir1/file1.txt",12L}, + {"read-tests/dir1/subdir1/file2.txt",12L}, + {"read-tests/dir1/subdir1/file3.txt",12L}, + //{"read-tests/largefile.txt", 3221225472L}, + {"read-tests/file space.txt",20L} + }).stream().collect(Collectors.toMap(o -> (String)o[0], o -> (Long)o[1])); + /** * Non-Delegating Class Loader. */ @@ -217,7 +249,17 @@ public void testSealing() throws Exception { @Test public void testThreadSafety() throws Exception { - final int THREADS = 40; + final FileSystemManager manager = getManager(); + + // The http4 and sftp mechanisms do not work with this thread safety test + List schemes = Arrays.asList(manager.getSchemes()); + if (schemes.contains("http4") || schemes.contains("sftp")) { + System.out.println("VfsClassLoaderTests skipping thread safety test for schemes :" + schemes); + return; + } + + // note THREADS must be an even number + final int THREADS = 20; final BlockingQueue workQueue = new ArrayBlockingQueue<>(THREADS * 2); final List exceptions = new ArrayList<>(); final Thread.UncaughtExceptionHandler handler = new Thread.UncaughtExceptionHandler() { @@ -229,9 +271,10 @@ public void uncaughtException(Thread t, Throwable e) { } }; final ThreadFactory factory = new ThreadFactory() { + private int count = 0; @Override public Thread newThread(Runnable r) { - Thread thread = new Thread(r, "VfsClassLoaderTests.testThreadSafety"); + Thread thread = new Thread(r, "VfsClassLoaderTests.testThreadSafety #" + count++); thread.setUncaughtExceptionHandler(handler); return thread; } @@ -247,9 +290,12 @@ public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) { }; final ThreadPoolExecutor executor = new ThreadPoolExecutor(THREADS, THREADS, 0, TimeUnit.SECONDS, workQueue, factory, rejectionHandler); executor.prestartAllCoreThreads(); - for (int i = 0; i < THREADS; i++) { + VFSClassLoader resourceLoader = createClassLoader(); + final CyclicBarrier barrier = new CyclicBarrier(THREADS); + for (int i = 0; i < THREADS/2; i++) { final VFSClassLoader loader = createClassLoader(); - workQueue.put(new VfsClassLoaderTests.LoadClass(loader)); + workQueue.put(new VfsClassLoaderTests.LoadClass(barrier, loader)); + workQueue.put(new VfsClassLoaderTests.ReadResource(barrier, resourceLoader)); } while (!workQueue.isEmpty()) { Thread.sleep(10); @@ -263,14 +309,14 @@ public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) { workQueue.addAll(rejected); } executor.shutdown(); - executor.awaitTermination(30, TimeUnit.SECONDS); + executor.awaitTermination(5, TimeUnit.MINUTES); assertEquals(THREADS, executor.getCompletedTaskCount()); if (!exceptions.isEmpty()) { StringBuilder exceptionMsg = new StringBuilder(); StringBuilderWriter writer = new StringBuilderWriter(exceptionMsg); PrintWriter pWriter = new PrintWriter(writer); for (Throwable t : exceptions) { - pWriter.write(t.getMessage()); + pWriter.write(String.valueOf(t.getMessage())); pWriter.write('\n'); t.printStackTrace(pWriter); pWriter.write('\n'); @@ -282,13 +328,17 @@ public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) { private class LoadClass implements Runnable { private final VFSClassLoader loader; - public LoadClass(VFSClassLoader loader) { + private final CyclicBarrier barrier; + + public LoadClass(CyclicBarrier barrier, VFSClassLoader loader) { + this.barrier = barrier; this.loader = loader; } @Override public void run() { try { + barrier.await(); final Class testClass = loader.findClass("code.ClassToLoad"); final Package pack = testClass.getPackage(); assertEquals("code", pack.getName()); @@ -298,7 +348,80 @@ public void run() { assertEquals("**PRIVATE**", testObject.toString()); } catch (ClassNotFoundException | InstantiationException | IllegalAccessException e) { throw new RuntimeException(e); + } catch (InterruptedException | BrokenBarrierException e) { + throw new RuntimeException(e); + } + } + } + + private class ReadResource implements Runnable { + private final VFSClassLoader loader; + private final CyclicBarrier barrier; + + public ReadResource(CyclicBarrier barrier, VFSClassLoader loader) { + this.barrier = barrier; + this.loader = loader; + } + + @Override + public void run() { + try { + barrier.await(); + List> files = new ArrayList<>(TEST_FILES.entrySet()); + Collections.shuffle(files); + for (int i = 0; i < 10; i++) { + for (Map.Entry file : files) { + testFindResource(file.getKey(), file.getValue()); + testGetResource(file.getKey(), file.getValue()); + testResourceAsStream(file.getKey(), file.getValue()); + } + } + } catch (InterruptedException | BrokenBarrierException e) { + throw new RuntimeException(e); + } + } + + private void testResourceAsStream(String file, long size) { + try { + try (InputStream stream = loader.getResourceAsStream(file)) { + if (stream == null) { + loader.getResourceAsStream(file); + } + readStream(file, stream, size); + } + } catch (Exception e) { + throw new RuntimeException("Failed to read " + file + " on thread " + Thread.currentThread(), e); + } + } + private void testGetResource(String file, long size) { + try { + URL url = loader.getResource(file); + try (InputStream stream = url.openStream()) { + readStream(file, stream, size); + } + } catch (Exception e) { + throw new RuntimeException("Failed to read " + file + " on thread " + Thread.currentThread(), e); + } + } + private void testFindResource(String file, long size) { + try { + URL url = loader.findResource(file); + try (InputStream stream = url.openStream()) { + readStream(file, stream, size); + } + } catch (Exception e) { + throw new RuntimeException("Failed to read " + file + " on thread " + Thread.currentThread(), e); + } + } + private void readStream(String file, InputStream stream, long size) throws IOException { + long length = 0; + byte[] bytes = new byte[1024]; + int readBytes = stream.read(bytes); + while (readBytes >= 0) { + length += readBytes; + readBytes = stream.read(bytes); } + assertEquals("Incorrect length for " + file + " on thread " + Thread.currentThread(), length, size); } } diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpProviderTestCase.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpProviderTestCase.java index c3be9ae5f3..55778b5580 100644 --- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpProviderTestCase.java +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpProviderTestCase.java @@ -29,6 +29,7 @@ import org.apache.commons.vfs2.FileSystemOptions; import org.apache.commons.vfs2.ProviderTestSuite; import org.apache.commons.vfs2.impl.DefaultFileSystemManager; +import org.apache.ftpserver.ConnectionConfigFactory; import org.apache.ftpserver.FtpServer; import org.apache.ftpserver.FtpServerFactory; import org.apache.ftpserver.command.CommandFactory; @@ -113,6 +114,14 @@ static void setUpClass(final String rootDirectory, final FileSystemFactory fileS if (commandFactory != null) { serverFactory.setCommandFactory(commandFactory); } + ConnectionConfigFactory configFactory = new ConnectionConfigFactory(); + configFactory.setMaxLogins(500); + configFactory.setMaxThreads(500); + configFactory.setMaxAnonymousLogins(500); + configFactory.setMaxLoginFailures(100); + configFactory.setAnonymousLoginEnabled(true); + configFactory.setLoginFailureDelay(1); + serverFactory.setConnectionConfig(configFactory.createConnectionConfig()); final ListenerFactory factory = new ListenerFactory(); // set the port of the listener factory.setPort(0); diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftps/AbstractFtpsProviderTestCase.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftps/AbstractFtpsProviderTestCase.java index ecf768595f..6757e525ad 100644 --- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftps/AbstractFtpsProviderTestCase.java +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftps/AbstractFtpsProviderTestCase.java @@ -29,6 +29,7 @@ import org.apache.commons.vfs2.FileSystemOptions; import org.apache.commons.vfs2.ProviderTestSuite; import org.apache.commons.vfs2.impl.DefaultFileSystemManager; +import org.apache.ftpserver.ConnectionConfigFactory; import org.apache.ftpserver.FtpServer; import org.apache.ftpserver.FtpServerFactory; import org.apache.ftpserver.ftplet.FtpException; @@ -152,6 +153,15 @@ synchronized static void setUpClass(final boolean implicit) throws FtpException // replace the default listener serverFactory.addListener(LISTENER_NAME, listenerFactory.createListener()); + ConnectionConfigFactory configFactory = new ConnectionConfigFactory(); + configFactory.setMaxLogins(1000); + configFactory.setMaxThreads(1000); + configFactory.setMaxAnonymousLogins(1000); + configFactory.setMaxLoginFailures(100); + configFactory.setAnonymousLoginEnabled(true); + configFactory.setLoginFailureDelay(1); + serverFactory.setConnectionConfig(configFactory.createConnectionConfig()); + // start the server EmbeddedFtpServer = serverFactory.createServer(); EmbeddedFtpServer.start(); @@ -221,8 +231,9 @@ public void prepare(final DefaultFileSystemManager manager) throws Exception { } protected void setupOptions(final FtpsFileSystemConfigBuilder builder) { - builder.setConnectTimeout(fileSystemOptions, Duration.ofSeconds(10)); - builder.setDataTimeout(fileSystemOptions, Duration.ofSeconds(10)); + builder.setConnectTimeout(fileSystemOptions, Duration.ofSeconds(60)); + builder.setDataTimeout(fileSystemOptions, Duration.ofSeconds(60)); + builder.setSoTimeout(fileSystemOptions, Duration.ofSeconds(60)); } }