Repository: netty/netty
Issue: [netty-5.0.0.Alpha1] ThreadLocalRandom.current() clears thread's interrupt status
Labels: defect

API method _io.netty.util.internal.ThreadLocalRandom.current()_ clears thread's interrupt status because _ThreadLocalRandom.getInitialSeedUniquifier()_ swallows _InterruptedException_ without restoring interrupt status (see https://github.com/netty/netty/blob/netty-5.0.0.Alpha1/common/src/main/java/io/netty/util/internal/ThreadLocalRandom.java#L114). Note that such behavior isn't specified neither in class _ThreadLocalRandom_ specification nor in method _ThreadLocalRandom.getInitialSeedUniquifier()_ specification.

This bug was introduced in netty-4.0.8.Final.

This bug also causes unexpected behavior of other netty API methods which invoke _ThreadLocalRandom.getInitialSeedUniquifier()_ (e.g. because of _ThreadLocalRandom.current()_ method invocation). One of such affected methods is _io.netty.bootstrap.AbstractBootstrap.bind(SocketAddress)_:

> Thread.currentThread().interrupt();
> assert Thread.currentThread().isInterrupted() == true; // OK
> ChannelFuture serverSocketChannelBindFuture = serverBootstrap.bind(address);
> assert Thread.currentThread().isInterrupted() == true; // FAIL
> serverSocketChannelBindFuture.await();
> assert serverSocketChannelBindFuture.isSuccess() == true; // OK if we comment out FAIL assert above


Issue comments:
Suggested fix:

instead of ignoring

> catch (InterruptedException ignore) {
>     // Ignore
> }

restore interrupt status

> catch (InterruptedException ignore) {
> Thread.currentThread().interrupt();
> }

Pull request with the suggested fix: https://github.com/netty/netty/pull/2171

Was fixed in 4.0 and master.. Thanks!


Pull request review comments:

Patch that resolved the issue:
diff --git a/common/src/main/java/io/netty/util/internal/ThreadLocalRandom.java b/common/src/main/java/io/netty/util/internal/ThreadLocalRandom.java
index 22ff7e47e18..3049e48274c 100644
--- a/common/src/main/java/io/netty/util/internal/ThreadLocalRandom.java
+++ b/common/src/main/java/io/netty/util/internal/ThreadLocalRandom.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2013 The Netty Project
+ * Copyright 2014 The Netty Project
  *
  * The Netty Project licenses this file to you under the Apache License,
  * version 2.0 (the "License"); you may not use this file except in compliance
@@ -110,8 +110,9 @@ public void run() {
                         initialSeedUniquifier = result;
                         break;
                     }
-                } catch (InterruptedException ignore) {
-                    // Ignore
+                } catch (InterruptedException e) {
+                    // restore interrupt status because we don't know how to/don't need to handle it here
+                    Thread.currentThread().interrupt();
                 }
             }
 


Test patch:
diff --git a/common/src/test/java/io/netty/util/internal/ThreadLocalRandomTest.java b/common/src/test/java/io/netty/util/internal/ThreadLocalRandomTest.java
new file mode 100644
index 00000000000..8f584d7f52d
--- /dev/null
+++ b/common/src/test/java/io/netty/util/internal/ThreadLocalRandomTest.java
@@ -0,0 +1,37 @@
+/*
+ * Copyright 2014 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.util.internal;
+
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class ThreadLocalRandomTest {
+
+    @Test
+    public void getInitialSeedUniquifierPreservesInterrupt() {
+        try {
+            Thread.currentThread().interrupt();
+            assertTrue("Assert that thread is interrupted before invocation of getInitialSeedUniquifier()",
+                    Thread.currentThread().isInterrupted());
+            ThreadLocalRandom.getInitialSeedUniquifier();
+            assertTrue("Assert that thread is interrupted after invocation of getInitialSeedUniquifier()",
+                    Thread.currentThread().isInterrupted());
+        } finally {
+            Thread.interrupted(); // clear interrupted status in order to not affect other tests
+        }
+    }
+}