Ensure interrupted flag is cleared after Timeout OpenLiberty#938

fmhwong · Sep 18, 2017 · 7a36ade · 7a36ade
1 parent a136d9b
commit 7a36ade
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 1 deletion.
diff --git a/...i_fat/fat/src/com/ibm/websphere/microprofile/faulttolerance_fat/tests/CDITimeoutTest.java b/...i_fat/fat/src/com/ibm/websphere/microprofile/faulttolerance_fat/tests/CDITimeoutTest.java
@@ -60,6 +60,18 @@ public void testTimeoutZero() throws Exception {
         getSharedServer().verifyResponse(browser, "/CDIFaultTolerance/timeout?testMethod=testTimeoutZero", "SUCCESS");
     }
 
+    @Test
+    public void testNonInterruptableTimeout() throws Exception {
+        WebBrowser browser = createWebBrowserForTestCase();
+        getSharedServer().verifyResponse(browser, "/CDIFaultTolerance/timeout?testMethod=testNonInterruptableTimeout", "SUCCESS");
+    }
+
+    @Test
+    public void testNonInterruptableDoesntTimeout() throws Exception {
+        WebBrowser browser = createWebBrowserForTestCase();
+        getSharedServer().verifyResponse(browser, "/CDIFaultTolerance/timeout?testMethod=testNonInterruptableDoesntTimeout", "SUCCESS");
+    }
+
     /** {@inheritDoc} */
     @Override
     protected SharedServer getSharedServer() {

diff --git a/...FaultTolerance.war/src/com/ibm/ws/microprofile/faulttolerance_fat/cdi/TimeoutServlet.java b/...FaultTolerance.war/src/com/ibm/ws/microprofile/faulttolerance_fat/cdi/TimeoutServlet.java
@@ -118,4 +118,27 @@ public void testTimeoutZero(HttpServletRequest request, HttpServletResponse resp
         // No TimeoutException expected
     }
 
+    public void testNonInterruptableTimeout() throws InterruptedException {
+        try {
+            bean.busyWait(1000); // Busy wait time is greater than timeout (=500)
+            fail("No exception thrown");
+        } catch (TimeoutException e) {
+            if (Thread.interrupted()) {
+                fail("Thread was in interrupted state upon return");
+            }
+            // This wait is to ensure our thread doesn't get interrupted later, after the method has finished
+            Thread.sleep(1000);
+        }
+    }
+
+    public void testNonInterruptableDoesntTimeout() throws Exception {
+        bean.busyWait(10); // Busy wait time is less than timeout (=500)
+
+        if (Thread.interrupted()) {
+            fail("Thread was in interrupted state upon return");
+        }
+
+        Thread.sleep(2000); // Wait to ensure that our thread isn't interrupted after the method has finished
+    }
+
 }
diff --git a/...ltTolerance.war/src/com/ibm/ws/microprofile/faulttolerance_fat/cdi/beans/TimeoutBean.java b/...ltTolerance.war/src/com/ibm/ws/microprofile/faulttolerance_fat/cdi/beans/TimeoutBean.java
@@ -10,6 +10,7 @@
  *******************************************************************************/
 package com.ibm.ws.microprofile.faulttolerance_fat.cdi.beans;
 
+import java.time.Duration;
 import java.time.temporal.ChronoUnit;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Future;
@@ -91,4 +92,18 @@ public int getConnectDCalls() {
     public void connectE() throws InterruptedException {
         Thread.sleep(2000);
     }
+
+    /**
+     * Used for testing timeout with workloads which are not interruptable
+     * 
+     * @param milliseconds number of milliseconds to busy wait for
+     */
+    @Timeout(500)
+    public void busyWait(int milliseconds) {
+        long duration = Duration.ofMillis(milliseconds).toNanos();
+        long start = System.nanoTime();
+        while (System.nanoTime() - start < duration) {
+            // Do nothing
+        }
+    }
 }
diff --git a/...roprofile.faulttolerance/src/com/ibm/ws/microprofile/faulttolerance/impl/TimeoutImpl.java b/...roprofile.faulttolerance/src/com/ibm/ws/microprofile/faulttolerance/impl/TimeoutImpl.java
@@ -238,9 +238,18 @@ public long check() {
         lock.readLock().lock();
         try {
             if (this.timedout) {
+                // Note: this clears the interrupted flag if it was set
+                // Assumption is that the interruption was caused by the Timeout
+                boolean wasInterrupted = Thread.interrupted();
+
                 if (TraceComponent.isAnyTracingEnabled() && tc.isDebugEnabled()) {
-                    Tr.debug(tc, "{0}: Throwing timeout exception", getDescriptor());
+                    if (wasInterrupted) {
+                        Tr.debug(tc, "{0}: Throwing timeout exception", getDescriptor());
+                    } else {
+                        Tr.debug(tc, "{0}: Throwing timeout exception and clearing interrupted flag", getDescriptor());
+                    }
                 }
+
                 throw new TimeoutException(Tr.formatMessage(tc, "timeout.occurred.CWMFT0000E"));
             }
             long now = System.nanoTime();