Skip to content
This repository has been archived by the owner on Sep 21, 2022. It is now read-only.

M2E support and fix resource leaks #124

Open
glassfishrobot opened this issue Feb 23, 2014 · 7 comments
Open

M2E support and fix resource leaks #124

glassfishrobot opened this issue Feb 23, 2014 · 7 comments

Comments

@glassfishrobot
Copy link
Contributor

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
Copy link
Contributor Author

Reported by atrajano

@glassfishrobot
Copy link
Contributor Author

Sub-Tasks:
JAX_WS_COMMONS-128

@glassfishrobot
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants