You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by kw...@apache.org on 2022/01/12 16:30:30 UTC
[jackrabbit-filevault] 01/01: JCRVLT-591 track unclosed archives
This is an automated email from the ASF dual-hosted git repository.
kwin pushed a commit to branch feature/JCRVLT-591-track-unclosed-packages
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git
commit 88780edfb55641e0ed134ac1b14f4a8c08ad86de
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Wed Jan 12 17:30:15 2022 +0100
JCRVLT-591 track unclosed archives
---
vault-core/bnd.bnd | 3 +-
vault-core/pom.xml | 6 +++
.../jackrabbit/vault/fs/io/AbstractArchive.java | 31 +++++++++++++
.../apache/jackrabbit/vault/fs/io/FileArchive.java | 2 +-
.../apache/jackrabbit/vault/fs/io/ZipArchive.java | 4 ++
.../jackrabbit/vault/fs/io/ZipNioArchive.java | 5 ++
.../jackrabbit/vault/fs/io/ZipStreamArchive.java | 7 +++
.../apache/jackrabbit/vault/fs/io/ArchiveTest.java | 53 +++++++++++++++++++---
.../packaging/integration/IntegrationTestBase.java | 21 +++++++--
9 files changed, 118 insertions(+), 14 deletions(-)
diff --git a/vault-core/bnd.bnd b/vault-core/bnd.bnd
index 5c5a0d4..87e44d7 100644
--- a/vault-core/bnd.bnd
+++ b/vault-core/bnd.bnd
@@ -22,7 +22,8 @@ DynamicImport-Package: *
-includeresource: @txw2-[0-9.]*.jar!/com/sun/xml/txw2/output/(IndentingXMLStreamWriter|DelegatingXMLStreamWriter).*,\
@woodstox-core-[0-9.]*.jar!/!module-info.class,\
@stax2-api-[0-9.]*.jar!/!module-info.class,\
- @maven-artifact-*.jar!/org/apache/maven/artifact/versioning/ComparableVersion*.class
+ @maven-artifact-[0-9.]*.jar!/org/apache/maven/artifact/versioning/ComparableVersion*.class,\
+ @h2-[0-9.]*.jar!/org/h2/util/CloseWatcher*.class
# whitelist the private reference usage in Packaging.getJcrPackageRegistry(Session)
-fixupmessages:"Export org.apache.jackrabbit.vault.packaging, has 1, private references [org.apache.jackrabbit.vault.packaging.registry.impl]"; \
restrict:=warning; \
diff --git a/vault-core/pom.xml b/vault-core/pom.xml
index 495994a..aec97a9 100644
--- a/vault-core/pom.xml
+++ b/vault-core/pom.xml
@@ -227,6 +227,12 @@
<artifactId>maven-artifact</artifactId>
<version>3.8.1</version>
</dependency>
+ <!-- for the https://github.com/h2database/h2database/blob/master/h2/src/main/org/h2/util/CloseWatcher.java -->
+ <dependency>
+ <groupId>com.h2database</groupId>
+ <artifactId>h2</artifactId>
+ <version>2.0.206</version>
+ </dependency>
<!-- test deps -->
<dependency>
<groupId>junit</groupId>
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/AbstractArchive.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/AbstractArchive.java
index 7deea69..0dd5066 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/AbstractArchive.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/AbstractArchive.java
@@ -20,6 +20,9 @@ package org.apache.jackrabbit.vault.fs.io;
import java.io.IOException;
import org.apache.jackrabbit.vault.util.Constants;
+import org.h2.util.CloseWatcher;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import org.apache.jackrabbit.util.Text;
/**
@@ -27,6 +30,13 @@ import org.apache.jackrabbit.util.Text;
*/
abstract class AbstractArchive implements Archive {
+ /**
+ * default logger
+ */
+ private static final Logger log = LoggerFactory.getLogger(AbstractArchive.class);
+
+ protected CloseWatcher watcher;
+
@Override
public Entry getEntry(String path) throws IOException {
String[] segs = Text.explode(path, '/');
@@ -50,4 +60,25 @@ abstract class AbstractArchive implements Archive {
Entry root = getEntry(rootPath);
return root == null ? null : new SubArchive(this, root, asJcrRoot);
}
+
+ static boolean dumpUnclosedArchives() {
+ boolean foundUnclosedArchive = false;
+ while (true) {
+ CloseWatcher w = CloseWatcher.pollUnclosed();
+ if (w == null) {
+ break;
+ }
+ foundUnclosedArchive = true;
+ log.error("Detected unclosed archive, it has been opened here\n: {}", w.getOpenStackTrace());
+ try {
+ AutoCloseable closeable = w.getCloseable();
+ if (closeable != null) {
+ closeable.close();
+ }
+ } catch (Exception e) {
+ log.error("Error forcing closing archive", e);
+ }
+ }
+ return foundUnclosedArchive;
+ }
}
\ No newline at end of file
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/FileArchive.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/FileArchive.java
index 52ff69d..c868b9a 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/FileArchive.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/FileArchive.java
@@ -33,7 +33,7 @@ import org.apache.jackrabbit.vault.util.Constants;
import org.apache.jackrabbit.vault.util.FileInputSource;
/**
- * Implements an archived based on the file system
+ * Implements an archive based on the file system
*/
public class FileArchive extends AbstractArchive {
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ZipArchive.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ZipArchive.java
index 22f4390..11843ba 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ZipArchive.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ZipArchive.java
@@ -36,6 +36,7 @@ import org.apache.jackrabbit.vault.fs.config.DefaultMetaInf;
import org.apache.jackrabbit.vault.fs.config.MetaInf;
import org.apache.jackrabbit.vault.fs.config.VaultSettings;
import org.apache.jackrabbit.vault.util.Constants;
+import org.h2.util.CloseWatcher;
import org.apache.jackrabbit.util.Text;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -148,6 +149,8 @@ public class ZipArchive extends AbstractArchive {
if (inf.getNodeTypes().isEmpty()) {
log.debug("Zip {} does not contain nodetypes.", file.getPath());
}
+ dumpUnclosedArchives();
+ watcher = CloseWatcher.register(this, jar, true);
}
@Override
@@ -217,6 +220,7 @@ public class ZipArchive extends AbstractArchive {
if (jar != null) {
jar.close();
jar = null;
+ CloseWatcher.unregister(watcher);
}
if (file != null && isTempFile) {
FileUtils.deleteQuietly(file);
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ZipNioArchive.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ZipNioArchive.java
index 6cf76fe..bfb2059 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ZipNioArchive.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ZipNioArchive.java
@@ -31,12 +31,14 @@ import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;
+import org.apache.commons.io.input.NullInputStream;
import org.apache.jackrabbit.vault.fs.api.VaultInputSource;
import org.apache.jackrabbit.vault.fs.config.ConfigurationException;
import org.apache.jackrabbit.vault.fs.config.DefaultMetaInf;
import org.apache.jackrabbit.vault.fs.config.MetaInf;
import org.apache.jackrabbit.vault.fs.config.VaultSettings;
import org.apache.jackrabbit.vault.util.Constants;
+import org.h2.util.CloseWatcher;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.slf4j.Logger;
@@ -98,6 +100,8 @@ public class ZipNioArchive extends AbstractArchive {
} catch (ProviderNotFoundException e) {
throw new IOException("Can not open zip file '" + path + "'", e);
}
+ dumpUnclosedArchives();
+ watcher = CloseWatcher.register(this, zipFileSystem, true);
}
@Override
@@ -193,6 +197,7 @@ public class ZipNioArchive extends AbstractArchive {
@Override
public void close() {
if (zipFileSystem != null) {
+ CloseWatcher.unregister(watcher);
try {
zipFileSystem.close();
} catch (IOException e) {
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ZipStreamArchive.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ZipStreamArchive.java
index a8cd148..b3a8497 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ZipStreamArchive.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ZipStreamArchive.java
@@ -32,12 +32,14 @@ import java.util.zip.ZipInputStream;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.IOUtils;
import org.apache.commons.io.input.CloseShieldInputStream;
+import org.apache.commons.io.input.NullInputStream;
import org.apache.jackrabbit.vault.fs.api.VaultInputSource;
import org.apache.jackrabbit.vault.fs.config.ConfigurationException;
import org.apache.jackrabbit.vault.fs.config.DefaultMetaInf;
import org.apache.jackrabbit.vault.fs.config.MetaInf;
import org.apache.jackrabbit.vault.fs.config.VaultSettings;
import org.apache.jackrabbit.vault.util.Constants;
+import org.h2.util.CloseWatcher;
import org.apache.jackrabbit.util.Text;
import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
@@ -184,6 +186,8 @@ public class ZipStreamArchive extends AbstractArchive {
log.debug("Zip stream does not contain nodetypes.");
}
}
+ dumpUnclosedArchives();
+ watcher = CloseWatcher.register(this, new NullInputStream(0), true);
}
/**
@@ -272,6 +276,9 @@ public class ZipStreamArchive extends AbstractArchive {
if (in != null) {
IOUtils.closeQuietly(in);
}
+ if (watcher != null) {
+ CloseWatcher.unregister(watcher);
+ }
if (raf != null) {
try {
raf.close();
diff --git a/vault-core/src/test/java/org/apache/jackrabbit/vault/fs/io/ArchiveTest.java b/vault-core/src/test/java/org/apache/jackrabbit/vault/fs/io/ArchiveTest.java
index 08bafc3..e94a1c7 100644
--- a/vault-core/src/test/java/org/apache/jackrabbit/vault/fs/io/ArchiveTest.java
+++ b/vault-core/src/test/java/org/apache/jackrabbit/vault/fs/io/ArchiveTest.java
@@ -20,9 +20,11 @@ import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeTrue;
import java.io.IOException;
import java.io.InputStream;
+import java.io.UncheckedIOException;
import java.net.URISyntaxException;
import java.nio.file.Path;
import java.nio.file.Paths;
@@ -38,9 +40,12 @@ import java.util.function.Supplier;
import org.apache.jackrabbit.vault.fs.api.VaultInputSource;
import org.apache.jackrabbit.vault.fs.config.DefaultMetaInf;
import org.apache.jackrabbit.vault.fs.io.Archive.Entry;
+import org.apache.jackrabbit.vault.packaging.integration.IntegrationTestBase;
import org.junit.After;
import org.junit.Before;
+import org.junit.ClassRule;
import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
@@ -48,12 +53,28 @@ import org.junit.runners.Parameterized.Parameters;
@RunWith(Parameterized.class)
public class ArchiveTest {
- @Parameters(name = "{1}" )
- public static Iterable<? extends Object[]> data() throws URISyntaxException {
+ @ClassRule
+ public static TemporaryFolder tempFolder = new TemporaryFolder();
+
+ @Parameters(name = "{2}" )
+ public static Iterable<? extends Object[]> data() throws URISyntaxException, IOException {
Path zipPath = Paths.get(ArchiveTest.class.getResource("/test-packages/atomic-counter-test.zip").toURI());
Supplier<Archive> zsaSupplier = () -> new ZipStreamArchive(ArchiveTest.class.getResourceAsStream("/test-packages/atomic-counter-test.zip"));
Supplier<Archive> zaSupplier = () -> new ZipArchive(zipPath.toFile());
Supplier<Archive> znaSupplier = () -> new ZipNioArchive(zipPath);
+ Supplier<Archive> faSupplier = () -> {
+ try {
+ return new FileArchive(IntegrationTestBase.getFile("/test-packages/tmp.zip", () -> {
+ try {
+ return tempFolder.newFile();
+ } catch (IOException e) {
+ throw new UncheckedIOException("cannot create temp file", e);
+ }
+ }));
+ } catch (IOException e) {
+ throw new UncheckedIOException("cannot create temp file", e);
+ }
+ };
Supplier<Archive> maSupplier = () -> {
MemoryArchive memoryArchive;
try (InputStream input = ArchiveTest.class.getResourceAsStream("/test-packages/atomic-counter-test.zip")) {
@@ -65,18 +86,23 @@ public class ArchiveTest {
return memoryArchive;
};
return Arrays.asList(new Object[][] {
- { zsaSupplier, "ZipStreamArchive" },
- { zaSupplier, "ZipArchive" },
- { znaSupplier, "ZipNioArchive" },
- { maSupplier, "MemoryArchive" }
+ { zsaSupplier, true, "ZipStreamArchive" },
+ { zaSupplier, true, "ZipArchive" },
+ { znaSupplier, true, "ZipNioArchive" },
+ { maSupplier, false, "MemoryArchive" }
+ //{ faSupplier, true, "FileArchive" }
+ // TODO: SubArchive, MappedArchive, JcrArchive
});
}
private final Supplier<Archive> archiveSupplier;
+ private final boolean hasCloseWatcher;
private Archive archive;
- public ArchiveTest(Supplier<Archive> archiveSupplier, String name) {
+
+ public ArchiveTest(Supplier<Archive> archiveSupplier, boolean hasCloseWatcher, String name) {
this.archiveSupplier = archiveSupplier;
+ this.hasCloseWatcher = hasCloseWatcher;
}
@Before
@@ -148,4 +174,17 @@ public class ArchiveTest {
public void testCloseWithoutOpen() {
archive.close();
}
+
+ private void openArchive() throws IOException {
+ archiveSupplier.get().open(false);
+ }
+
+ @Test
+ public void testDumpForForgottenClose() throws IOException, InterruptedException {
+ assumeTrue("Skipping test as this archive has no CloseWatcher", hasCloseWatcher);
+ openArchive();
+ System.gc();
+ Thread.sleep(500);
+ assertTrue(AbstractArchive.dumpUnclosedArchives());
+ }
}
diff --git a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/IntegrationTestBase.java b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/IntegrationTestBase.java
index 8feb266..03e7fe9 100644
--- a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/IntegrationTestBase.java
+++ b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/IntegrationTestBase.java
@@ -27,6 +27,7 @@ import static org.junit.Assert.fail;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
+import java.io.UncheckedIOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.file.Files;
@@ -42,6 +43,7 @@ import java.util.Map;
import java.util.Objects;
import java.util.Properties;
import java.util.Set;
+import java.util.function.Supplier;
import java.util.jar.JarFile;
import javax.jcr.Node;
@@ -373,8 +375,8 @@ public class IntegrationTestBase {
}
}
- public InputStream getStream(String name) {
- return Objects.requireNonNull(getClass().getResourceAsStream(name), "Could not find class resource with name '" + name + "'");
+ public static InputStream getStream(String name) {
+ return Objects.requireNonNull(IntegrationTestBase.class.getResourceAsStream(name), "Could not find class resource with name '" + name + "'");
}
/**
@@ -384,14 +386,24 @@ public class IntegrationTestBase {
* @throws IOException
*/
public File getFile(String name) throws IOException {
+ return getFile(name, () -> {
+ try {
+ return tempFolder.newFile();
+ } catch (IOException e) {
+ throw new UncheckedIOException("cannot create temp file", e);
+ }
+ });
+ }
+
+ public static File getFile(String name, Supplier<File> tmpFileSupplier) throws IOException {
URI uri;
try {
- uri = Objects.requireNonNull(getClass().getResource(name), "Could not find class resource with name '" + name + "'").toURI();
+ uri = Objects.requireNonNull(IntegrationTestBase.class.getResource(name), "Could not find class resource with name '" + name + "'").toURI();
} catch (URISyntaxException e) {
throw new IOException("Could not convert class resource URL to URI", e);
}
if (uri.isOpaque()) { // non hierarchical URIs (for resources in a JAR) can not use classical file operations
- File tmpFile = tempFolder.newFile();
+ File tmpFile = tmpFileSupplier.get();
try (InputStream in = getStream(name)) {
Files.copy(in, tmpFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
}
@@ -400,7 +412,6 @@ public class IntegrationTestBase {
return new File(uri);
}
}
-
public Archive getFileArchive(String name) throws IOException {
final File file = getFile(name);
if (file.isDirectory()) {