In case you missed it, there is one System.gc() not in the if condition.
As System.gc() should in theory never been called, I would try to minimize
the number of calls (performance, memory, etc side effects).

What do you think?

--
Jean-Louis Monteiro
http://twitter.com/jlouismonteiro
http://www.tomitribe.com

On Thu, Mar 12, 2015 at 10:07 AM, <[email protected]> wrote:

> Repository: tomee
> Updated Branches:
>   refs/heads/master 53c704852 -> bda2f2767
>
>
> Workaround for Windows bug JDK-4715154
>
>
> Project: http://git-wip-us.apache.org/repos/asf/tomee/repo
> Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/bda2f276
> Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/bda2f276
> Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/bda2f276
>
> Branch: refs/heads/master
> Commit: bda2f276764926022db730c5d145a7d5add450b0
> Parents: 53c7048
> Author: [email protected] <[email protected]>
> Authored: Thu Mar 12 10:07:25 2015 +0100
> Committer: [email protected] <[email protected]>
> Committed: Thu Mar 12 10:07:42 2015 +0100
>
> ----------------------------------------------------------------------
>  .../openejb/loader/BasicURLClassPath.java       |  2 +-
>  .../java/org/apache/openejb/loader/Files.java   | 41 +++++++++++++++++++-
>  .../loader/provisining/MavenResolverTest.java   | 12 +++---
>  3 files changed, 47 insertions(+), 8 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/tomee/blob/bda2f276/container/openejb-loader/src/main/java/org/apache/openejb/loader/BasicURLClassPath.java
> ----------------------------------------------------------------------
> diff --git
> a/container/openejb-loader/src/main/java/org/apache/openejb/loader/BasicURLClassPath.java
> b/container/openejb-loader/src/main/java/org/apache/openejb/loader/BasicURLClassPath.java
> index e0d4837..b4c8af1 100644
> ---
> a/container/openejb-loader/src/main/java/org/apache/openejb/loader/BasicURLClassPath.java
> +++
> b/container/openejb-loader/src/main/java/org/apache/openejb/loader/BasicURLClassPath.java
> @@ -76,7 +76,7 @@ public abstract class BasicURLClassPath implements
> ClassPath {
>          });
>
>          final URL[] jars = new URL[jarNames.length];
> -        final boolean isWindows = System.getProperty("os.name",
> "unknown").toLowerCase().startsWith("windows");
> +        final boolean isWindows = System.getProperty("os.name",
> "unknown").toLowerCase().startsWith("win");
>
>          for (int j = 0; j < jarNames.length; j++) {
>              final String name = isWindows ? jarNames[j].toLowerCase() :
> jarNames[j];
>
>
> http://git-wip-us.apache.org/repos/asf/tomee/blob/bda2f276/container/openejb-loader/src/main/java/org/apache/openejb/loader/Files.java
> ----------------------------------------------------------------------
> diff --git
> a/container/openejb-loader/src/main/java/org/apache/openejb/loader/Files.java
> b/container/openejb-loader/src/main/java/org/apache/openejb/loader/Files.java
> index 1b0e6f8..04793f4 100644
> ---
> a/container/openejb-loader/src/main/java/org/apache/openejb/loader/Files.java
> +++
> b/container/openejb-loader/src/main/java/org/apache/openejb/loader/Files.java
> @@ -44,6 +44,7 @@ import static
> org.apache.openejb.loader.JarLocation.decode;
>   */
>  public class Files {
>      private static final Map<String, MessageDigest> DIGESTS = new
> HashMap<String, MessageDigest>();
> +    private static final boolean isWindows = System.getProperty("os.name",
> "unknown").toLowerCase().startsWith("win");
>
>      public static File path(final String... parts) {
>          File dir = null;
> @@ -119,6 +120,8 @@ public class Files {
>          if (!file.isDirectory()) {
>              throw new FileRuntimeException("Not a directory: " +
> file.getAbsolutePath());
>          }
> +
> +        System.gc();
>          return file;
>      }
>
> @@ -167,6 +170,12 @@ public class Files {
>          if (file.exists()) {
>              return file;
>          }
> +
> +        if (isWindows) {
> +            //Known Windows bug JDK-4715154 and as of JDK8 still not
> fixable due to OS
> +            System.gc();
> +        }
> +
>          if (!file.mkdirs()) {
>              throw new FileRuntimeException("Cannot mkdir: " +
> file.getAbsolutePath());
>          }
> @@ -179,12 +188,18 @@ public class Files {
>
>      public static File tmpdir() {
>          try {
> -            File file = null;
> +            File file;
>              try {
>                  file = File.createTempFile("temp", "dir");
>              } catch (final Throwable e) {
>                  //Use a local tmp directory
>                  final File tmp = new File("tmp");
> +
> +                if (isWindows) {
> +                    //Known Windows bug JDK-4715154 and as of JDK8 still
> not fixable due to OS
> +                    System.gc();
> +                }
> +
>                  if (!tmp.exists() && !tmp.mkdirs()) {
>                      throw new IOException("Failed to create local tmp
> directory: " + tmp.getAbsolutePath());
>                  }
> @@ -192,6 +207,11 @@ public class Files {
>                  file = File.createTempFile("temp", "dir", tmp);
>              }
>
> +            if (isWindows) {
> +                //Known Windows bug JDK-4715154 and as of JDK8 still not
> fixable due to OS
> +                System.gc();
> +            }
> +
>              if (!file.delete()) {
>                  throw new IOException("Failed to create temp dir. Delete
> failed");
>              }
> @@ -215,6 +235,11 @@ public class Files {
>
>          if (!file.exists()) {
>
> +            if (isWindows) {
> +                //Known Windows bug JDK-4715154 and as of JDK8 still not
> fixable due to OS
> +                System.gc();
> +            }
> +
>              if (!file.mkdirs()) {
>                  throw new FileRuntimeException("Cannot mkdirs: " +
> file.getAbsolutePath());
>              }
> @@ -275,6 +300,7 @@ public class Files {
>      }
>
>      public static void delete(final File file) {
> +
>          if (file.exists()) {
>              if (file.isDirectory()) {
>                  final File[] files = file.listFiles();
> @@ -285,6 +311,12 @@ public class Files {
>                  }
>              }
>              try {
> +
> +                if (isWindows) {
> +                    //Known Windows bug JDK-4715154 and as of JDK8 still
> not fixable due to OS
> +                    System.gc();
> +                }
> +
>                  if (!file.delete()) {
>                      file.deleteOnExit();
>                  }
> @@ -295,6 +327,7 @@ public class Files {
>      }
>
>      public static void remove(final File file) {
> +
>          if (file == null) {
>              return;
>          }
> @@ -310,6 +343,12 @@ public class Files {
>                  }
>              }
>          }
> +
> +        if (isWindows) {
> +            //Known Windows bug JDK-4715154 and as of JDK8 still not
> fixable due to OS
> +            System.gc();
> +        }
> +
>          if (!file.delete()) {
>              throw new IllegalStateException("Could not delete file: " +
> file.getAbsolutePath());
>          }
>
>
> http://git-wip-us.apache.org/repos/asf/tomee/blob/bda2f276/container/openejb-loader/src/test/java/org/apache/openejb/loader/provisining/MavenResolverTest.java
> ----------------------------------------------------------------------
> diff --git
> a/container/openejb-loader/src/test/java/org/apache/openejb/loader/provisining/MavenResolverTest.java
> b/container/openejb-loader/src/test/java/org/apache/openejb/loader/provisining/MavenResolverTest.java
> index 9988bcc..7d005c1 100644
> ---
> a/container/openejb-loader/src/test/java/org/apache/openejb/loader/provisining/MavenResolverTest.java
> +++
> b/container/openejb-loader/src/test/java/org/apache/openejb/loader/provisining/MavenResolverTest.java
> @@ -45,10 +45,10 @@ public class MavenResolverTest {
>
>      @Test
>      public void resolve() throws Exception {
> -        File file = new File("target/test/foo1.jar");
> +        final File file = new File("target/test/foo.jar");
>          Files.remove(file);
>          Files.mkdirs(file.getParentFile());
> -        FileOutputStream to = new FileOutputStream(file);
> +        final FileOutputStream to = new FileOutputStream(file);
>          IO.copy(resolver.resolve("mvn:junit:junit:4.12:jar"), to);
>          IO.close(to);
>          assertTrue(file.exists());
> @@ -57,10 +57,10 @@ public class MavenResolverTest {
>
>      @Test
>      public void customRepo() throws Exception {
> -        File file = new File("target/test/foo2.jar");
> +        final File file = new File("target/test/foo.jar");
>          Files.remove(file);
>          Files.mkdirs(file.getParentFile());
> -        FileOutputStream to = new FileOutputStream(file);
> +        final FileOutputStream to = new FileOutputStream(file);
>          IO.copy(resolver.resolve("mvn:
> http://repo1.maven.org/maven2/!junit:junit:4.12:jar";), to);
>          IO.close(to);
>          assertTrue(file.exists());
> @@ -69,10 +69,10 @@ public class MavenResolverTest {
>
>      @Test
>      public void latest() throws Exception {
> -        File file = new File("target/test/foo3.jar");
> +        final File file = new File("target/test/foo.jar");
>          Files.remove(file);
>          Files.mkdirs(file.getParentFile());
> -        FileOutputStream to = new FileOutputStream(file);
> +        final FileOutputStream to = new FileOutputStream(file);
>          IO.copy(resolver.resolve("mvn:
> http://repo1.maven.org/maven2/!junit:junit:LATEST:jar";), to);
>          IO.close(to);
>          assertTrue(file.exists());
>
>

Reply via email to