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:29 UTC
[jackrabbit-filevault] branch feature/JCRVLT-591-track-unclosed-packages created (now 88780ed)
This is an automated email from the ASF dual-hosted git repository.
kwin pushed a change to branch feature/JCRVLT-591-track-unclosed-packages
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git.
at 88780ed JCRVLT-591 track unclosed archives
This branch includes the following new commits:
new 88780ed JCRVLT-591 track unclosed archives
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[jackrabbit-filevault] 01/01: JCRVLT-591 track unclosed archives
Posted by kw...@apache.org.
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()) {