[WFCORE-5606] Refactor resource closing (server)

Fixes https://issues.redhat.com/browse/WFCORE-5606 as part of https://issues.redhat.com/browse/WFCORE-5606
wildfly · Oct 9, 2021 · 23a8f78 · 23a8f78
1 parent ed84b32
commit 23a8f78
Show file tree

Hide file tree

Showing 25 changed files with 125 additions and 143 deletions.
diff --git a/.../common/src/main/resources/modules/system/layers/base/org/jboss/as/server/main/module.xml b/.../common/src/main/resources/modules/system/layers/base/org/jboss/as/server/main/module.xml
@@ -83,6 +83,7 @@
         <module name="org.jboss.stdio"/>
         <module name="org.jboss.threads"/>
         <module name="org.jboss.vfs" services="import"/>
+        <module name="org.jboss.xnio"/>
         <module name="org.jboss.as.controller"/>
         <module name="org.jboss.as.deployment-repository"/>
         <module name="org.jboss.as.domain-http-interface"/>

diff --git a/server/src/main/java/org/jboss/as/server/controller/git/GitRepository.java b/server/src/main/java/org/jboss/as/server/controller/git/GitRepository.java
@@ -68,11 +68,11 @@
 public class GitRepository implements Closeable {
 
     private final Set<String> ignored;
-    private final Repository repository;
+    private Repository repository;
     private final Path basePath;
     private final String defaultRemoteRepository;
     private final String branch;
-    private final SshdSessionFactory sshdSessionFactory;
+    private SshdSessionFactory sshdSessionFactory;
 
     public GitRepository(GitRepositoryConfiguration gitConfig)
             throws IllegalArgumentException, IOException, ConfigXMLParseException, GeneralSecurityException {
@@ -266,10 +266,33 @@ public boolean isBare() {
 
     @Override
     public void close() {
-        if (sshdSessionFactory != null) {
-            this.sshdSessionFactory.close();
+        SshdSessionFactory toCloseSshdSessionFactory = sshdSessionFactory;
+        sshdSessionFactory = null;
+        RuntimeException rex = null;
+        if (toCloseSshdSessionFactory != null) {
+            try {
+                toCloseSshdSessionFactory.close();
+            } catch (RuntimeException e) {
+                rex = e;
+            }
+        }
+
+        Repository toCloseRepository = repository;
+        repository = null;
+        if (toCloseRepository != null) {
+            try {
+                toCloseRepository.close();
+            } catch (RuntimeException e) {
+                if (rex == null) {
+                    rex = e;
+                } else {
+                    rex.addSuppressed(e);
+                }
+            }
+        }
+        if (rex != null) {
+            throw rex;
         }
-        this.repository.close();
     }
 
     public String getPattern(File file) {

diff --git a/server/src/main/java/org/jboss/as/server/deployment/AbstractDeploymentUploadHandler.java b/server/src/main/java/org/jboss/as/server/deployment/AbstractDeploymentUploadHandler.java
@@ -18,6 +18,8 @@
  */
 package org.jboss.as.server.deployment;
 
+import static org.xnio.IoUtils.safeClose;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.EnumSet;
@@ -79,14 +81,4 @@ public void execute(OperationContext context, ModelNode operation) throws Operat
 
     protected abstract InputStream getContentInputStream(OperationContext context, ModelNode operation) throws IOException, OperationFailedException;
 
-    private static void safeClose(InputStream is) {
-        if (is != null) {
-            try {
-                is.close();
-            }
-            catch (Exception e) {
-                ServerLogger.ROOT_LOGGER.caughtExceptionClosingContentInputStream(e);
-            }
-        }
-    }
 }
diff --git a/server/src/main/java/org/jboss/as/server/deployment/DeploymentAddHandler.java b/server/src/main/java/org/jboss/as/server/deployment/DeploymentAddHandler.java
@@ -36,6 +36,7 @@
 import static org.jboss.as.server.deployment.DeploymentHandlerUtils.asString;
 import static org.jboss.as.server.deployment.DeploymentHandlerUtils.getInputStream;
 import static org.jboss.as.server.deployment.DeploymentHandlerUtils.hasValidContentAdditionParameterDefined;
+import static org.xnio.IoUtils.safeClose;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -49,7 +50,6 @@
 import org.jboss.as.controller.ProcessType;
 import org.jboss.as.controller.descriptions.ModelDescriptionConstants;
 import org.jboss.as.controller.registry.Resource;
-import org.jboss.as.protocol.StreamUtils;
 import org.jboss.as.repository.ContentRepository;
 import org.jboss.as.server.logging.ServerLogger;
 import org.jboss.dmr.ModelNode;
@@ -205,16 +205,13 @@ DeploymentHandlerUtil.ContentItem addFromContentAdditionParameter(OperationConte
         InputStream in = getInputStream(context, contentItemNode);
         InputStream transformed = null;
         try {
-            try {
-                transformed = deploymentTransformation.doTransformation(context, contentItemNode, name, in);
-                hash = contentRepository.addContent(transformed);
-            } catch (IOException e) {
-                throw createFailureException(e.toString());
-            }
-
+            transformed = deploymentTransformation.doTransformation(context, contentItemNode, name, in);
+            hash = contentRepository.addContent(transformed);
+        } catch (IOException e) {
+            throw createFailureException(e.toString());
         } finally {
-            StreamUtils.safeClose(in);
-            StreamUtils.safeClose(transformed);
+            safeClose(in);
+            safeClose(transformed);
         }
         return new DeploymentHandlerUtil.ContentItem(hash);
     }

diff --git a/server/src/main/java/org/jboss/as/server/deployment/DeploymentFullReplaceHandler.java b/server/src/main/java/org/jboss/as/server/deployment/DeploymentFullReplaceHandler.java
@@ -34,6 +34,7 @@
 import static org.jboss.as.server.deployment.DeploymentHandlerUtils.createFailureException;
 import static org.jboss.as.server.deployment.DeploymentHandlerUtils.getInputStream;
 import static org.jboss.as.server.deployment.DeploymentHandlerUtils.hasValidContentAdditionParameterDefined;
+import static org.xnio.IoUtils.safeClose;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -47,7 +48,6 @@
 import org.jboss.as.controller.PathAddress;
 import org.jboss.as.controller.PathElement;
 import org.jboss.as.controller.registry.Resource;
-import org.jboss.as.protocol.StreamUtils;
 import org.jboss.as.repository.ContentReference;
 import org.jboss.as.repository.ContentRepository;
 import org.jboss.as.server.controller.resources.DeploymentAttributes;
@@ -204,16 +204,13 @@ DeploymentHandlerUtil.ContentItem addFromContentAdditionParameter(OperationConte
         InputStream in = getInputStream(context, contentItemNode);
         InputStream transformed = null;
         try {
-            try {
-                transformed = deploymentTransformation.doTransformation(context, contentItemNode, name, in);
-                hash = contentRepository.addContent(transformed);
-            } catch (IOException e) {
-                throw createFailureException(e.toString());
-            }
-
+            transformed = deploymentTransformation.doTransformation(context, contentItemNode, name, in);
+            hash = contentRepository.addContent(transformed);
+        } catch (IOException e) {
+            throw createFailureException(e.toString());
         } finally {
-            StreamUtils.safeClose(in);
-            StreamUtils.safeClose(transformed);
+            safeClose(in);
+            safeClose(transformed);
         }
         contentItemNode.clear(); // AS7-1029
         contentItemNode.get(CONTENT_HASH.getName()).set(hash);

diff --git a/server/src/main/java/org/jboss/as/server/deployment/DeploymentMountProvider.java b/server/src/main/java/org/jboss/as/server/deployment/DeploymentMountProvider.java
@@ -23,6 +23,7 @@
 package org.jboss.as.server.deployment;
 
 import static java.security.AccessController.doPrivileged;
+import static org.xnio.IoUtils.safeClose;
 
 import java.io.Closeable;
 import java.io.IOException;
@@ -45,7 +46,6 @@
 import org.jboss.threads.JBossThreadFactory;
 import org.jboss.vfs.TempFileProvider;
 import org.jboss.vfs.VFS;
-import org.jboss.vfs.VFSUtils;
 import org.jboss.vfs.VirtualFile;
 
 /**
@@ -139,7 +139,8 @@ public void stop(final StopContext context) {
                     public void run() {
                         try {
                             deploymentMountProviderConsumer.accept(null);
-                            VFSUtils.safeClose(tempFileProvider);
+                            safeClose(tempFileProvider);
+                            tempFileProvider = null; //not needed anymore
                         } finally {
                             try {
                                 ScheduledExecutorService ses = scheduledExecutorService;

diff --git a/...rc/main/java/org/jboss/as/server/deployment/DeploymentOverlayDeploymentUnitProcessor.java b/...rc/main/java/org/jboss/as/server/deployment/DeploymentOverlayDeploymentUnitProcessor.java
@@ -233,11 +233,7 @@ protected static void copyFile(final InputStream in, final File dest) throws IOE
         }
     }
 
-
     protected static void close(Closeable closeable) {
-        try {
-            closeable.close();
-        } catch (IOException ignore) {
-        }
+        org.xnio.IoUtils.safeClose(closeable);
     }
 }
diff --git a/server/src/main/java/org/jboss/as/server/deployment/MountedDeploymentOverlay.java b/server/src/main/java/org/jboss/as/server/deployment/MountedDeploymentOverlay.java
@@ -1,9 +1,10 @@
 package org.jboss.as.server.deployment;
 
+import static org.xnio.IoUtils.safeClose;
+
 import org.jboss.vfs.TempFileProvider;
 import org.jboss.vfs.VFS;
 import org.jboss.vfs.VirtualFile;
-import org.xnio.IoUtils;
 
 import java.io.Closeable;
 import java.io.File;
@@ -27,7 +28,8 @@ public MountedDeploymentOverlay(Closeable closeable, File realFile, VirtualFile
     }
 
     public void remountAsZip(boolean expanded) throws IOException {
-        IoUtils.safeClose(closeable);
+        safeClose(closeable);
+        closeable = null; // following assignments may fail
         if(expanded) {
             closeable = VFS.mountZipExpanded(realFile, mountPoint, tempFileProvider);
         } else {
@@ -37,6 +39,8 @@ public void remountAsZip(boolean expanded) throws IOException {
 
     @Override
     public void close() throws IOException {
-        closeable.close();
+        Closeable toClose = closeable;
+        closeable = null;
+        toClose.close();
     }
 }
diff --git a/server/src/main/java/org/jboss/as/server/deployment/ServiceLoaderProcessor.java b/server/src/main/java/org/jboss/as/server/deployment/ServiceLoaderProcessor.java
@@ -22,6 +22,8 @@
 
 package org.jboss.as.server.deployment;
 
+import static org.xnio.IoUtils.safeClose;
+
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStream;
@@ -35,7 +37,6 @@
 import org.jboss.as.server.logging.ServerLogger;
 import org.jboss.as.server.deployment.module.ModuleRootMarker;
 import org.jboss.as.server.deployment.module.ResourceRoot;
-import org.jboss.vfs.VFSUtils;
 import org.jboss.vfs.VirtualFile;
 
 /**
@@ -91,7 +92,7 @@ private void processRoot(final ResourceRoot resourceRoot, final Map<String, List
                         list.add(className);
                     }
                 } finally {
-                    VFSUtils.safeClose(stream);
+                    safeClose(stream);
                 }
             } catch (IOException e) {
                 throw ServerLogger.ROOT_LOGGER.failedToReadVirtualFile(child, e);

diff --git a/server/src/main/java/org/jboss/as/server/deployment/annotation/ResourceRootIndexer.java b/server/src/main/java/org/jboss/as/server/deployment/annotation/ResourceRootIndexer.java
@@ -27,6 +27,8 @@
 import java.util.List;
 import java.util.Set;
 
+import static org.xnio.IoUtils.safeClose;
+
 import org.jboss.as.server.logging.ServerLogger;
 import org.jboss.as.server.deployment.Attachments;
 import org.jboss.as.server.deployment.DeploymentUnitProcessingException;
@@ -35,7 +37,6 @@
 import org.jboss.jandex.Index;
 import org.jboss.jandex.IndexReader;
 import org.jboss.jandex.Indexer;
-import org.jboss.vfs.VFSUtils;
 import org.jboss.vfs.VirtualFile;
 import org.jboss.vfs.VirtualFileFilter;
 import org.jboss.vfs.VisitorAttributes;
@@ -100,7 +101,7 @@ public boolean accepts(VirtualFile file) {
                 } catch (Exception e) {
                     ServerLogger.DEPLOYMENT_LOGGER.cannotIndexClass(classFile.getPathNameRelativeTo(virtualFile), virtualFile.getPathName(), e);
                 } finally {
-                    VFSUtils.safeClose(inputStream);
+                    safeClose(inputStream);
                 }
             }
             final Index index = indexer.complete();

diff --git a/server/src/main/java/org/jboss/as/server/deployment/integration/Seam2Processor.java b/server/src/main/java/org/jboss/as/server/deployment/integration/Seam2Processor.java
@@ -22,6 +22,8 @@
 
 package org.jboss.as.server.deployment.integration;
 
+import static org.xnio.IoUtils.safeClose;
+
 import java.io.Closeable;
 import java.io.File;
 import java.net.URL;
@@ -55,7 +57,6 @@
 import org.jboss.msc.service.StartException;
 import org.jboss.msc.service.StopContext;
 import org.jboss.vfs.VFS;
-import org.jboss.vfs.VFSUtils;
 import org.jboss.vfs.VirtualFile;
 
 /**
@@ -107,7 +108,7 @@ public void start(StartContext startContext) throws StartException {
                     }
 
                     public void stop(StopContext stopContext) {
-                        VFSUtils.safeClose(mountHandle);
+                        safeClose(mountHandle);
                     }
 
                     public Closeable getValue() throws IllegalStateException, IllegalArgumentException {

diff --git a/...src/main/java/org/jboss/as/server/deployment/jbossallxml/JBossAllXMLParsingProcessor.java b/...src/main/java/org/jboss/as/server/deployment/jbossallxml/JBossAllXMLParsingProcessor.java
@@ -22,7 +22,8 @@
 
 package org.jboss.as.server.deployment.jbossallxml;
 
-import java.io.Closeable;
+import static org.xnio.IoUtils.safeClose;
+
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
@@ -147,32 +148,15 @@ private void parse(final InputStream source, final File file,final XMLMapper map
             try {
                 mapper.parseDocument(context, streamReader);
             } finally {
-                safeClose(streamReader);
+                if (streamReader != null) {
+                    safeClose((AutoCloseable) streamReader::close);
+                }
             }
         } catch (XMLStreamException e) {
             throw ServerLogger.ROOT_LOGGER.errorLoadingJBossXmlFile(file.getPath(), e);
         }
     }
 
-    private static void safeClose(final Closeable closeable) {
-        if (closeable != null)
-            try {
-                closeable.close();
-            } catch (IOException e) {
-                // ignore
-            }
-    }
-
-    private static void safeClose(final XMLStreamReader streamReader) {
-        if (streamReader != null)
-            try {
-                streamReader.close();
-            } catch (XMLStreamException e) {
-                // ignore
-            }
-    }
-
-
     private static class Parser implements XMLElementReader<JBossAllXmlParseContext> {
 
         public static final Parser INSTANCE = new Parser();

diff --git a/server/src/main/java/org/jboss/as/server/deployment/module/DeploymentRootMountProcessor.java b/server/src/main/java/org/jboss/as/server/deployment/module/DeploymentRootMountProcessor.java
@@ -22,6 +22,8 @@
 
 package org.jboss.as.server.deployment.module;
 
+import static org.xnio.IoUtils.safeClose;
+
 import java.io.Closeable;
 import java.io.IOException;
 
@@ -36,7 +38,6 @@
 import org.jboss.as.server.deployment.MountExplodedMarker;
 import org.jboss.as.server.deployment.MountType;
 import org.jboss.vfs.VFS;
-import org.jboss.vfs.VFSUtils;
 import org.jboss.vfs.VirtualFile;
 
 /**
@@ -94,8 +95,8 @@ public void deploy(DeploymentPhaseContext phaseContext) throws DeploymentUnitPro
                 failed = true;
                 throw ServerLogger.ROOT_LOGGER.deploymentMountFailed(e);
             } finally {
-                if(failed) {
-                    VFSUtils.safeClose(handle);
+                if (failed) {
+                    safeClose(handle);
                 }
             }
         }
@@ -109,7 +110,7 @@ public void undeploy(DeploymentUnit context) {
         final ResourceRoot resourceRoot = context.removeAttachment(Attachments.DEPLOYMENT_ROOT);
         if (resourceRoot != null) {
             final Closeable mountHandle = resourceRoot.getMountHandle();
-            VFSUtils.safeClose(mountHandle);
+            safeClose(mountHandle);
         }
     }
 }