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()); > >
