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 2021/06/07 18:48:01 UTC
[jackrabbit-filevault] branch
bugfix/JCRVLT-531-cleanup-hooks-properly created (now 6a7bbf5)
This is an automated email from the ASF dual-hosted git repository.
kwin pushed a change to branch bugfix/JCRVLT-531-cleanup-hooks-properly
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git.
at 6a7bbf5 JCRVLT-531 always clean up hooks
This branch includes the following new commits:
new 6a7bbf5 JCRVLT-531 always clean up hooks
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-531 always clean up hooks
Posted by kw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
kwin pushed a commit to branch bugfix/JCRVLT-531-cleanup-hooks-properly
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git
commit 6a7bbf536467d2b572925f05ae72c554f1d1eb22
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Mon Jun 7 20:47:41 2021 +0200
JCRVLT-531 always clean up hooks
---
.../vault/packaging/InstallHookProcessor.java | 10 ++-
.../packaging/impl/InstallHookProcessorImpl.java | 76 +++++++++++++---------
.../vault/packaging/impl/ZipVaultPackage.java | 61 ++++++++++-------
.../jackrabbit/vault/packaging/package-info.java | 2 +-
4 files changed, 93 insertions(+), 56 deletions(-)
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/InstallHookProcessor.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/InstallHookProcessor.java
index 3a0f6a4..1915442 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/InstallHookProcessor.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/InstallHookProcessor.java
@@ -16,6 +16,7 @@
*/
package org.apache.jackrabbit.vault.packaging;
+import java.io.Closeable;
import java.io.IOException;
import org.apache.jackrabbit.vault.fs.api.VaultInputSource;
@@ -24,7 +25,7 @@ import org.apache.jackrabbit.vault.fs.io.Archive;
/**
* The install hook processor is used for handle the install hooks, from registration to execution.
*/
-public interface InstallHookProcessor {
+public interface InstallHookProcessor extends Closeable {
/**
* Register all hooks found in the given archive.
@@ -55,4 +56,11 @@ public interface InstallHookProcessor {
* @return {@code true} if successful.
*/
boolean execute(InstallContext context);
+
+ /**
+ * Cleans up any registered hooks
+ */
+ @Override
+ default void close() throws IOException {
+ }
}
\ No newline at end of file
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/InstallHookProcessorImpl.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/InstallHookProcessorImpl.java
index 9137ec5..69e41c6 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/InstallHookProcessorImpl.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/InstallHookProcessorImpl.java
@@ -17,20 +17,20 @@
package org.apache.jackrabbit.vault.packaging.impl;
-import java.io.File;
import java.io.IOException;
import java.io.InputStream;
-import java.io.OutputStream;
import java.net.URL;
import java.net.URLClassLoader;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
import java.util.Enumeration;
import java.util.Properties;
import java.util.TreeMap;
import java.util.jar.JarFile;
import java.util.jar.Manifest;
-import org.apache.commons.io.FileUtils;
-import org.apache.commons.io.IOUtils;
+import org.apache.jackrabbit.util.Text;
import org.apache.jackrabbit.vault.fs.api.VaultInputSource;
import org.apache.jackrabbit.vault.fs.io.Archive;
import org.apache.jackrabbit.vault.packaging.InstallContext;
@@ -39,7 +39,6 @@ import org.apache.jackrabbit.vault.packaging.InstallHookProcessor;
import org.apache.jackrabbit.vault.packaging.PackageException;
import org.apache.jackrabbit.vault.packaging.VaultPackage;
import org.apache.jackrabbit.vault.util.Constants;
-import org.apache.jackrabbit.util.Text;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -53,7 +52,7 @@ public class InstallHookProcessorImpl implements InstallHookProcessor {
*/
private static final Logger log = LoggerFactory.getLogger(InstallHookProcessorImpl.class);
- private final TreeMap<String, Hook> hooks = new TreeMap<String, Hook>();
+ private final TreeMap<String, Hook> hooks = new TreeMap<>();
public void registerHooks(Archive archive, ClassLoader classLoader) throws PackageException {
try {
@@ -104,14 +103,17 @@ public class InstallHookProcessorImpl implements InstallHookProcessor {
public void registerHook(VaultInputSource input, ClassLoader classLoader) throws IOException, PackageException {
// first we need to spool the jar file to disk.
- File jarFile = File.createTempFile("vaulthook", ".jar");
+ Path jarFile = Files.createTempFile("vaulthook", ".jar");
Hook hook = new Hook(input.getSystemId(), jarFile, classLoader);
- try (OutputStream out = FileUtils.openOutputStream(jarFile);
- InputStream in = input.getByteStream()) {
- IOUtils.copy(in, out);
+ try (InputStream in = input.getByteStream()) {
+ Files.copy(in, jarFile, StandardCopyOption.REPLACE_EXISTING);
} catch (IOException e) {
- hook.destroy();
+ try {
+ hook.destroy();
+ } catch (IOException ioeDuringDestroy) {
+ e.addSuppressed(ioeDuringDestroy);
+ }
throw e;
}
initHook(hook);
@@ -120,13 +122,13 @@ public class InstallHookProcessorImpl implements InstallHookProcessor {
private void initHook(Hook hook) throws IOException, PackageException {
try {
hook.init();
- } catch (IOException e) {
- log.error("Error while initializing hook: {}", e.toString());
- hook.destroy();
- throw e;
- } catch (PackageException e) {
+ } catch (IOException|PackageException e) {
log.error("Error while initializing hook: {}", e.toString());
- hook.destroy();
+ try {
+ hook.destroy();
+ } catch (IOException ioeDuringDestroy) {
+ e.addSuppressed(ioeDuringDestroy);
+ }
throw e;
}
hooks.put(hook.name, hook);
@@ -144,24 +146,38 @@ public class InstallHookProcessorImpl implements InstallHookProcessor {
} catch (Throwable e) {
// abort processing only for prepare and installed phase
if (context.getPhase() == InstallContext.Phase.PREPARE || context.getPhase() == InstallContext.Phase.INSTALLED) {
- log.warn("Hook " + hook.name +" threw exception. {} aborted.", context.getPhase(), e);
+ log.warn("Hook {} threw exception. {} aborted.", hook.name, context.getPhase(), e);
return false;
}
- log.warn("Hook " + hook.name +" threw exception. Ignored", e);
+ log.warn("Hook {} threw exception. Ignored", hook.name, e);
}
- // if in end phase, shutdown hooks
- if (context.getPhase() == InstallContext.Phase.END) {
+ }
+ return true;
+ }
+
+ @Override
+ public void close() throws IOException {
+ IOException ioException = null;
+ for (Hook hook : hooks.values()) {
+ try {
hook.destroy();
+ } catch (IOException e) {
+ if (ioException == null) {
+ ioException = new IOException("Error while destroying one or more hooks. Look at suppressed exceptions for details!");
+ }
+ ioException.addSuppressed(e);
}
}
- return true;
+ if (ioException != null) {
+ throw ioException;
+ }
}
private class Hook {
private final String name;
- private final File jarFile;
+ private final Path jarFile;
private ClassLoader parentClassLoader;
@@ -176,17 +192,17 @@ public class InstallHookProcessorImpl implements InstallHookProcessor {
this.jarFile = null;
}
- private Hook(String name, File jarFile, ClassLoader parentClassLoader) {
+ private Hook(String name, Path jarFile, ClassLoader parentClassLoader) {
this.name = name;
this.jarFile = jarFile;
this.parentClassLoader = parentClassLoader;
}
- private void destroy() {
+ private void destroy() throws IOException {
parentClassLoader = null;
hook = null;
if (jarFile != null) {
- FileUtils.deleteQuietly(jarFile);
+ Files.deleteIfExists(jarFile);
}
}
@@ -194,7 +210,7 @@ public class InstallHookProcessorImpl implements InstallHookProcessor {
try {
if (jarFile != null) {
// open jar file and extract classname from manifest
- try (JarFile jar = new JarFile(jarFile)) {
+ try (JarFile jar = new JarFile(jarFile.toFile())) {
Manifest mf = jar.getManifest();
if (mf == null) {
throw new PackageException("hook jar file does not have a manifest: " + name);
@@ -209,17 +225,17 @@ public class InstallHookProcessorImpl implements InstallHookProcessor {
try {
// 1st fallback is the current classes classloader (the bundle classloader in the OSGi context)
loadMainClass(URLClassLoader.newInstance(
- new URL[] { jarFile.toURI().toURL() },
+ new URL[] { jarFile.toUri().toURL() },
this.getClass().getClassLoader()));
} catch (ClassNotFoundException cnfe) {
// 2nd fallback is the thread context classloader
loadMainClass(URLClassLoader.newInstance(
- new URL[] { jarFile.toURI().toURL() },
+ new URL[] { jarFile.toUri().toURL() },
Thread.currentThread().getContextClassLoader()));
}
} else {
loadMainClass(URLClassLoader.newInstance(
- new URL[] { jarFile.toURI().toURL() },
+ new URL[] { jarFile.toUri().toURL() },
parentClassLoader));
}
} else {
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/ZipVaultPackage.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/ZipVaultPackage.java
index d07dbdc..9e91635 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/ZipVaultPackage.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/ZipVaultPackage.java
@@ -192,33 +192,41 @@ public class ZipVaultPackage extends PackagePropertiesImpl implements VaultPacka
InstallHookProcessor hooks = opts instanceof InstallHookProcessorFactory ?
((InstallHookProcessorFactory) opts).createInstallHookProcessor()
: new InstallHookProcessorImpl();
- if (!opts.isDryRun()) {
- hooks.registerHooks(archive, opts.getHookClassLoader());
- }
-
- checkAllowanceToInstallPackage(session, hooks, securityConfig);
-
- // check for disable intermediate saves (JCRVLT-520)
- if (Boolean.parseBoolean(getProperty(PackageProperties.NAME_DISABLE_INTERMEDIATE_SAVE))) {
- // MAX_VALUE disables saving completely, therefore we have to use a lower value!
- opts.setAutoSaveThreshold(Integer.MAX_VALUE - 1);
- }
-
- Importer importer = new Importer(opts, isStrictByDefault);
- AccessControlHandling ac = getACHandling();
- if (opts.getAccessControlHandling() == null) {
- opts.setAccessControlHandling(ac);
- }
- String cndPattern = getProperty(NAME_CND_PATTERN);
- if (cndPattern != null) {
+ try {
+ if (!opts.isDryRun()) {
+ hooks.registerHooks(archive, opts.getHookClassLoader());
+ }
+ checkAllowanceToInstallPackage(session, hooks, securityConfig);
+
+ // check for disable intermediate saves (JCRVLT-520)
+ if (Boolean.parseBoolean(getProperty(PackageProperties.NAME_DISABLE_INTERMEDIATE_SAVE))) {
+ // MAX_VALUE disables saving completely, therefore we have to use a lower value!
+ opts.setAutoSaveThreshold(Integer.MAX_VALUE - 1);
+ }
+
+ Importer importer = new Importer(opts, isStrictByDefault);
+ AccessControlHandling ac = getACHandling();
+ if (opts.getAccessControlHandling() == null) {
+ opts.setAccessControlHandling(ac);
+ }
+ String cndPattern = getProperty(NAME_CND_PATTERN);
+ if (cndPattern != null) {
+ try {
+ opts.setCndPattern(cndPattern);
+ } catch (PatternSyntaxException e) {
+ throw new PackageException("Specified CND pattern not valid.", e);
+ }
+ }
+
+ return new InstallContextImpl(session, "/", this, importer, hooks);
+ } catch (RepositoryException|PackageException e) {
try {
- opts.setCndPattern(cndPattern);
- } catch (PatternSyntaxException e) {
- throw new PackageException("Specified CND pattern not valid.", e);
+ hooks.close();
+ } catch (IOException exceptionDuringClose) {
+ e.addSuppressed(exceptionDuringClose);
}
+ throw e;
}
-
- return new InstallContextImpl(session, "/", this, importer, hooks);
}
protected void checkAllowanceToInstallPackage(@NotNull Session session, @NotNull InstallHookProcessor hookProcessor, @NotNull AbstractPackageRegistry.SecurityConfig securityConfig) throws PackageException, RepositoryException {
@@ -279,6 +287,11 @@ public class ZipVaultPackage extends PackagePropertiesImpl implements VaultPacka
} finally {
ctx.setPhase(InstallContext.Phase.END);
hooks.execute(ctx);
+ try {
+ hooks.close();
+ } catch (IOException e) {
+ log.warn("Error closing hook processor", e);
+ }
}
if (subPackages != null) {
subPackages.addAll(importer.getSubPackages());
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/package-info.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/package-info.java
index 0c8c0f8..1d4a580 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/package-info.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/package-info.java
@@ -15,7 +15,7 @@
* limitations under the License.
*/
-@Version("2.13.0")
+@Version("2.14.0")
package org.apache.jackrabbit.vault.packaging;
import org.osgi.annotation.versioning.Version;