javaee / metro-jaxws-commons

Metro has been contributed to Eclipse Foundation. This repository is for legacy review only. Please refer to the Eclipse EE4J Metro project for the very latest
https://eclipse-ee4j.github.io/metro-wsit/
Other
10 stars 9 forks source link

M2E support and fix resource leaks #124

Open glassfishrobot opened 10 years ago

glassfishrobot commented 10 years ago

There are some resource leak warnings brought up when using Eclipse IDE and ideally this should work with M2E.

Affected Versions

[2.3.1]

glassfishrobot commented 10 years ago

Reported by atrajano

glassfishrobot commented 10 years ago

atrajano said: Not sure how to send a patch so just putting it here for now. The idea for doing this is in http://www.trajano.net/2013/12/jaxws-maven-plugin-and-m2e/

diff --git a/src/main/java/org/jvnet/jax_ws_commons/jaxws/WsImportMojo.java b/src/main/java/org/jvnet/jax_ws_commons/jaxws/WsImportMojo.java
index 3bd6003..113c5cb 100644
--- a/src/main/java/org/jvnet/jax_ws_commons/jaxws/WsImportMojo.java
+++ b/src/main/java/org/jvnet/jax_ws_commons/jaxws/WsImportMojo.java
@@ -57,6 +57,7 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+
 import org.apache.maven.artifact.Artifact;
 import org.apache.maven.plugin.MojoExecutionException;
 import org.apache.maven.plugins.annotations.Parameter;
@@ -540,9 +541,11 @@ abstract class WsImportMojo extends AbstractJaxwsMojo
 Logger.getLogger(WsImportMojo.class.getName()).log(Level.SEVERE, null, ex);
             }
         }
+        // Suppress this warning because it is actually being closed if the type is URLClassLoader
+        @SuppressWarnings("resource")
         ClassLoader loader = urlCpath.isEmpty()
 ? Thread.currentThread().getContextClassLoader()
-: new URLClassLoader(urlCpath.toArray(new URL[urlCpath.size()]));
+: new URLClassLoader(urlCpath.toArray(new URL[0]));
         if (wsdlFiles != null) {
             for (String wsdlFileName : wsdlFiles) {
 File wsdl = new File(wsdlFileName);
@@ -593,7 +596,8 @@ abstract class WsImportMojo extends AbstractJaxwsMojo
     String path = u.getPath();
     try {
         Pattern p = Pattern.compile(dir.replace(File.separatorChar, '/') + PATTERN, Pattern.CASE_INSENSITIVE);
-        Enumeration<JarEntry> jes = new JarFile(path.substring(5, path.indexOf("!/"))).entries();
+        JarFile jarFile = new JarFile(path.substring(5, path.indexOf("!/")));
+        Enumeration<JarEntry> jes = jarFile.entries();
         while (jes.hasMoreElements()) {
             JarEntry je = jes.nextElement();
             Matcher m = p.matcher(je.getName());
@@ -602,13 +606,21 @@ abstract class WsImportMojo extends AbstractJaxwsMojo
 files.add(new URL(s));
             }
         }
+        jarFile.close();
     } catch (IOException ex) {
         Logger.getLogger(WsImportMojo.class.getName()).log(Level.SEVERE, null, ex);
     }
 }
             }
         }
-        return files.toArray(new URL[files.size()]);
+        if (loader instanceof URLClassLoader) {
+            try {
+((URLClassLoader) loader).close();
+            } catch (IOException e) {
+throw new MojoExecutionException(e.getMessage(), e);
+            }
+        }
+        return files.toArray(new URL[0]);
     }

     /**
@@ -720,9 +732,9 @@ abstract class WsImportMojo extends AbstractJaxwsMojo
     }

     private String getHash(String s) {
+        Formatter formatter = new Formatter();
         try {
             MessageDigest md = MessageDigest.getInstance("SHA");
-            Formatter formatter = new Formatter();
             for (byte b : md.digest(s.getBytes("UTF-8"))) {
 formatter.format("%02x", b);
             }
@@ -731,6 +743,8 @@ abstract class WsImportMojo extends AbstractJaxwsMojo
             getLog().debug(ex.getMessage(), ex);
         } catch (NoSuchAlgorithmException ex) {
             getLog().debug(ex.getMessage(), ex);
+        } finally {
+            formatter.close();
         }
         //fallback to some default
         getLog().warn("Could not compute hash for " + s + ". Using fallback method.");
diff --git a/src/main/resources/META-INF/m2e/lifecycle-mapping-metadata.xml b/src/main/resources/META-INF/m2e/lifecycle-mapping-metadata.xml
new file mode 100644
index 0000000..85f6cc3
--- /dev/null
+++ b/src/main/resources/META-INF/m2e/lifecycle-mapping-metadata.xml
@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<lifecycleMappingMetadata>
+  <pluginExecutions>
+    <pluginExecution>
+      <pluginExecutionFilter>
+        <goals>
+          <goal>wsimport</goal>
+          <goal>wsgen</goal>
+          </goals>
+      </pluginExecutionFilter>
+      <action>
+        <execute>
+          <runOnIncremental>true</runOnIncremental>
+          <runOnConfiguration>true</runOnConfiguration>
+        </execute>
+      </action>
+    </pluginExecution>
+  </pluginExecutions>
+</lifecycleMappingMetadata>
diff --git a/src/test/java/org/jvnet/jax_ws_commons/jaxws/Assertions.java b/src/test/java/org/jvnet/jax_ws_commons/jaxws/Assertions.java
index ed5bf17..78a5289 100644
--- a/src/test/java/org/jvnet/jax_ws_commons/jaxws/Assertions.java
+++ b/src/test/java/org/jvnet/jax_ws_commons/jaxws/Assertions.java
@@ -46,11 +46,15 @@ public class Assertions {
         Assert.assertTrue(f.exists(), f.getAbsolutePath() + " does not exist");
         Assert.assertTrue(f.isFile(), f.getAbsolutePath() + " is not a file");
         BufferedReader r = new BufferedReader(new FileReader(f));
-        String line;
-        while ((line = r.readLine()) != null) {
-            if (line.contains(s)) {
-return;
+        try {
+            String line;
+            while ((line = r.readLine()) != null) {
+if (line.contains(s)) {
+    return;
+}
             }
+        } finally {
+            r.close();
         }
         Assert.fail("'" + s + "' is missing in:" + f.getAbsolutePath());
     }
@@ -61,6 +65,7 @@ public class Assertions {
         Assert.assertTrue(f.isFile(), f.getAbsolutePath() + " is not a file");
         ZipFile zf = new ZipFile(f);
         Assert.assertNotNull(zf.getEntry(path), "'" + path + "' is missing in: " + jarName);
+        zf.close();
     }

     public static void assertJarNotContains(File project, String jarName, String path) throws ZipException, IOException {
@@ -69,5 +74,6 @@ public class Assertions {
         Assert.assertTrue(f.isFile(), f.getAbsolutePath() + " is not a file");
         ZipFile zf = new ZipFile(f);
         Assert.assertNull(zf.getEntry(path), "'" + path + "' is in: " + jarName);
+        zf.close();
     }
 }
glassfishrobot commented 10 years ago

atrajano said: One note I only tested this on wsimport, I don't use wsgen personally, but I don't see why it won't work.

glassfishrobot commented 10 years ago

ahammar said: I suggest this ticket be split into two: One for the resource leaks and one for the m2e support. @atrajano: Could you please create a separate ticket for the resource leaks with a separate patch for fixing that?

Wrt the m2e support, I believe the necessary work is a bit greater than suggested. What needs to be done is described here: http://wiki.eclipse.org/M2E_compatible_maven_plugins

I've implemented this support for the jaxb2 and the sqlj maven plugins of Codehaus Mojo, and I offer to help out here and provide a patch. But I would like to get a confirmation that you would accept this addition before I put the required time into doing so.

glassfishrobot commented 10 years ago

ahammar said: The URLClassLoader.close() used in the patch is only available in Java 7+, and the plugin supports 6+. So it shouldn't be added.

glassfishrobot commented 10 years ago

Sub-Tasks: JAX_WS_COMMONS-128

glassfishrobot commented 7 years ago

This issue was imported from java.net JIRA JAX_WS_COMMONS-124