Merge pull request junit-team#549 from Tibor17/junit.reopen

fixes for junit-team#544 and junit-team#545

src/main/java/org/junit/internal/runners/statements/FailOnTimeout.java

@@ -2,6 +2,8 @@
 
 import org.junit.runners.model.Statement;
 
+import java.io.InterruptedIOException;
+import java.nio.channels.ClosedByInterruptException;
 import java.util.concurrent.TimeUnit;
 
 public class FailOnTimeout extends Statement {
@@ -29,6 +31,8 @@ public void evaluate() throws Throwable {
 
     private StatementThread evaluateStatement() throws InterruptedException {
         StatementThread thread = new StatementThread(fOriginalStatement);
+        // Let the process/application complete after timeout expired.
+        thread.setDaemon(true);
         thread.start();
         fTimeUnit.timedJoin(thread, fTimeout);
         if (!thread.fFinished) {
@@ -55,15 +59,27 @@ private void throwTimeoutException(StatementThread thread) throws Exception {
     }
 
     private static class StatementThread extends Thread {
+        /**
+         * This is final variable because the statement is set once.
+         * Final makes sure that the statement is immediately visible in
+         * #run() (other than current thread) after constructor finished.
+         */
         private final Statement fStatement;
 
-        private boolean fFinished = false;
+        /**
+         * These two variables are volatile to make sure that the Thread calling #evaluate()
+         * can immediately read their values set by this thread.
+         * */
+        private volatile boolean fFinished;
+        private volatile Throwable fExceptionThrownByOriginalStatement;
 
-        private Throwable fExceptionThrownByOriginalStatement = null;
-
-        private StackTraceElement[] fRecordedStackTrace = null;
+        // No need for volatile, because written and read by one thread.
+        private StackTraceElement[] fRecordedStackTrace;
 
         public StatementThread(Statement statement) {
+            fFinished = false;
+            fExceptionThrownByOriginalStatement = null;
+            fRecordedStackTrace = null;
             fStatement = statement;
         }
 
@@ -82,6 +98,10 @@ public void run() {
                 fFinished = true;
             } catch (InterruptedException e) {
                 // don't log the InterruptedException
+            } catch (InterruptedIOException e) {
+                // don't log the InterruptedIOException
+            } catch (ClosedByInterruptException e) {
+                // don't log the ClosedByInterruptException
             } catch (Throwable e) {
                 fExceptionThrownByOriginalStatement = e;
             }

src/test/java/org/junit/tests/experimental/rules/TimeoutRuleTest.java

@@ -4,11 +4,19 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 import org.junit.rules.TestRule;
 import org.junit.rules.Timeout;
 import org.junit.runner.JUnitCore;
 import org.junit.runner.Result;
 
+import java.io.File;
+import java.io.IOException;
+import java.io.InterruptedIOException;
+import java.io.RandomAccessFile;
+import java.nio.ByteBuffer;
+import java.nio.channels.FileChannel;
+import java.util.Random;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.ReentrantLock;
 
@@ -24,6 +32,9 @@ public class TimeoutRuleTest {
     public abstract static class AbstractTimeoutTest {
         public static final StringBuffer logger = new StringBuffer();
 
+        @Rule
+        public final TemporaryFolder tmpFile = new TemporaryFolder();
+
         @Test
         public void run1() throws InterruptedException {
             logger.append("run1");
@@ -49,6 +60,30 @@ public void run4() {
             while (!run4done) {
             }
         }
+
+        @Test
+        public void run5() throws IOException {
+            logger.append("run5");
+            Random rnd = new Random();
+            byte[] data = new byte[1024];
+            File tmp = tmpFile.newFile();
+            while (true) {
+                FileChannel channel = new RandomAccessFile(tmp, "rw").getChannel();
+                rnd.nextBytes(data);
+                ByteBuffer buffer = ByteBuffer.wrap(data);
+                // Interrupted thread closes channel and throws ClosedByInterruptException.
+                channel.write(buffer);
+                channel.close();
+                tmp.delete();
+            }
+        }
+
+        @Test
+        public void run6() throws InterruptedIOException {
+            logger.append("run6");
+            // Java IO throws InterruptedIOException only on SUN machines.
+            throw new InterruptedIOException();
+        }
     }
 
     public static class HasGlobalLongTimeout extends AbstractTimeoutTest {
@@ -71,29 +106,34 @@ public void before() {
 
     @After
     public void after() {
-        run4done = true;//to make sure that the thread won't continue at run4()
+        // set run4done to make sure that the thread won't continue at run4()
+        run4done = true;
         run1Lock.unlock();
     }
 
     @Test
     public void timeUnitTimeout() throws InterruptedException {
         HasGlobalTimeUnitTimeout.logger.setLength(0);
         Result result = JUnitCore.runClasses(HasGlobalTimeUnitTimeout.class);
-        assertEquals(4, result.getFailureCount());
+        assertEquals(6, result.getFailureCount());
         assertThat(HasGlobalTimeUnitTimeout.logger.toString(), containsString("run1"));
         assertThat(HasGlobalTimeUnitTimeout.logger.toString(), containsString("run2"));
         assertThat(HasGlobalTimeUnitTimeout.logger.toString(), containsString("run3"));
         assertThat(HasGlobalTimeUnitTimeout.logger.toString(), containsString("run4"));
+        assertThat(HasGlobalTimeUnitTimeout.logger.toString(), containsString("run5"));
+        assertThat(HasGlobalTimeUnitTimeout.logger.toString(), containsString("run6"));
     }
 
     @Test
     public void longTimeout() throws InterruptedException {
         HasGlobalLongTimeout.logger.setLength(0);
         Result result = JUnitCore.runClasses(HasGlobalLongTimeout.class);
-        assertEquals(4, result.getFailureCount());
+        assertEquals(6, result.getFailureCount());
         assertThat(HasGlobalLongTimeout.logger.toString(), containsString("run1"));
         assertThat(HasGlobalLongTimeout.logger.toString(), containsString("run2"));
         assertThat(HasGlobalLongTimeout.logger.toString(), containsString("run3"));
         assertThat(HasGlobalLongTimeout.logger.toString(), containsString("run4"));
+        assertThat(HasGlobalLongTimeout.logger.toString(), containsString("run5"));
+        assertThat(HasGlobalLongTimeout.logger.toString(), containsString("run6"));
     }
 }