Yes JL, not sure why I missed one? I will fix tonight. No Romain, just reverting will not fix the issue at all as the gc call is a known workaround for Win only and there will never be a fix for this. It is a known Windows IO issue for delete, mkdirs and mkdir in the OS - JDK-4715154 is just one of the first and clear references, but there are many more related issues that focus on the buffers that are not flushed until gc is called - Feel free to also search for similar issues if you have time. The gc call is only made on the broken OS so it will not affect other systems - Telling customers to switch to another OS or runtime is not impossible, but you can try if you like ;-) - I'll send you some 50 or so company addresses to get you started. There was no previous fix? If what you are referring to is the use of different files in the test, well that is not a fix, that's a one time workaround.
Yes Mark, as mentioned on IRC I have seen this before on Win platforms. Not sure how many times 'Files.remove' etc. is actually called during runtime, but it's impact is minimal. The test now passes consistently using the same file on Win platforms. I tested on Win7+8 using JDK 6+7+8 just to be sure. I was able to reproduce on Win7+8 on JDK6+8 as is, but writing just a simple del+create+file loop test will hit this on any jdk/win combination pretty consistently - the gc eliminates it. And this topic just forced me to bench it just now. In 100 cycles the gc adds +- <10ms on my system per run when compared to using 100 new files (ie. the gc call using the same file name is sometimes actually faster than creating new 100 files). Andy. On 15 March 2015 at 19:55, Mark Struberg <[email protected]> wrote: > >> > - File file = new File("target/test/foo2.jar"); > >> > + final File file = new File("target/test/foo.jar"); > > Andy, did you also look at the Resolution of JDK-4715154? > It will not be fixed. As far as I've seen it (only?) happens if you have > WindowsDefender running. > > I'm not quite sure if the gc() call really helps in all situations, but > using different files did. > > LieGrue, > strub > > > > > > > On Thursday, 12 March 2015, 14:58, Romain Manni-Bucau < > [email protected]> wrote: > > > Id revert it using a fixed jvm - we shoulfnt rely on gc at all + you > > reverted fixes from Mark so please correct it. > > Le 12 mars 2015 11:36, "Jean-Louis Monteiro" > > <[email protected]> a > > écrit : > > > > > >> 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);d > >> > 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()); > >> > > >> > > >> > > >
