Merge pull request #391 from /issues/387-nio-deadlock

Issues/387 nio deadlock
sniffy · Nov 10, 2020 · 36548d9 · 36548d9
2 parents 3637fa1 + fac7b0b
commit 36548d9
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 36 deletions.
diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml
@@ -71,7 +71,7 @@ jobs:
         with:
           java-version: 8
       - name: Build with Maven
-        run: mvn -B clean install --file pom.xml -U -P ci -DskipTests=true '-Dgpg.skip=true'
+        run: mvn -B clean install --file pom.xml -U -P ci '-Dgpg.skip=true'
       - name: Run compatibility tests using Java 8
         run: mvn -f sniffy-compatibility-tests/pom.xml test
       - name: Set up JDK 11

diff --git a/sniffy-integration-tests/sniffy-integration-test-tomcat/pom.xml b/sniffy-integration-tests/sniffy-integration-test-tomcat/pom.xml
@@ -57,6 +57,7 @@
                     <port>8081</port>
                     <path>/test</path>
                     <fork>true</fork>
+                    <useSeparateTomcatClassLoader>true</useSeparateTomcatClassLoader>
                     <useTestClasspath>true</useTestClasspath>
                 </configuration>
                 <executions>

diff --git a/sniffy-module-nio/src/main/java/io/sniffy/nio/SniffySelector.java b/sniffy-module-nio/src/main/java/io/sniffy/nio/SniffySelector.java
@@ -25,8 +25,8 @@ public class SniffySelector extends AbstractSelector {
 
     private final AbstractSelector delegate;
 
-    private final Map<AbstractSelectableChannel, AbstractSelectableChannel>
-            channelToSniffyChannelMap = new WeakHashMap<AbstractSelectableChannel, AbstractSelectableChannel>();
+    private final Map<AbstractSelectableChannel, AbstractSelectableChannel> channelToSniffyChannelMap =
+            Collections.synchronizedMap(new WeakHashMap<AbstractSelectableChannel, AbstractSelectableChannel>());
 
     private final Map<SelectionKey, SniffySelectionKey> sniffySelectionKeyCache =
             new WeakHashMap<SelectionKey, SniffySelectionKey>();
@@ -71,10 +71,8 @@ private Set<SelectionKey> wrapSelectionKeys(final Set<SelectionKey> delegates) {
 
                 @Override
                 public SniffySelectionKey wrap(SelectionKey delegate) {
-                    synchronized (channelToSniffyChannelMap) {
-                        //noinspection SuspiciousMethodCalls
-                        return SniffySelector.this.wrap(delegate, SniffySelector.this, channelToSniffyChannelMap.get(delegate.channel()));
-                    }
+                    //noinspection SuspiciousMethodCalls
+                    return SniffySelector.this.wrap(delegate, SniffySelector.this, channelToSniffyChannelMap.get(delegate.channel()));
                 }
 
             });
@@ -113,9 +111,7 @@ protected SelectionKey register(AbstractSelectableChannel ch, int ops, Object at
 
             if (ch instanceof SelectableChannelWrapper) {
                 chDelegate = ((SelectableChannelWrapper<?>) ch).getDelegate();
-                synchronized (channelToSniffyChannelMap) {
-                    channelToSniffyChannelMap.put(chDelegate, ch);
-                }
+                channelToSniffyChannelMap.put(chDelegate, ch);
             }
 
             SelectionKey selectionKeyDelegate = invokeMethod(AbstractSelector.class, delegate, "register",
@@ -166,45 +162,48 @@ public int selectNow() throws IOException {
      * select method can remove cancelled selection keys from delegate so we need to update them in sniffy channels as well
      */
     private void updateSelectionKeysFromDelegate() {
-        synchronized (channelToSniffyChannelMap) {
-            for (Map.Entry<AbstractSelectableChannel, AbstractSelectableChannel> entry : channelToSniffyChannelMap.entrySet()) {
-                AbstractSelectableChannel sniffyChannel = entry.getValue();
 
-                if (sniffyChannel instanceof SelectableChannelWrapper) {
-                    //((SniffySocketChannelAdapter) sniffyChannel).updateSelectionKeysFromDelegate(this);
-                    AbstractSelectableChannel delegate = ((SelectableChannelWrapper<?>) sniffyChannel).getDelegate();
-                    //noinspection TryWithIdenticalCatches
-                    try {
+        Map<AbstractSelectableChannel, AbstractSelectableChannel> channelToSniffyChannelMap;
+        synchronized (this.channelToSniffyChannelMap) {
+            channelToSniffyChannelMap = new HashMap<AbstractSelectableChannel, AbstractSelectableChannel>(this.channelToSniffyChannelMap);
+        }
+
+        for (Map.Entry<AbstractSelectableChannel, AbstractSelectableChannel> entry : channelToSniffyChannelMap.entrySet()) {
+            AbstractSelectableChannel sniffyChannel = entry.getValue();
+
+            if (sniffyChannel instanceof SelectableChannelWrapper) {
+                AbstractSelectableChannel delegate = ((SelectableChannelWrapper<?>) sniffyChannel).getDelegate();
+                try {
+
+                    Object keyLock = ReflectionUtil.getField(AbstractSelectableChannel.class, sniffyChannel, "keyLock");
+                    Object delegateKeyLock = ReflectionUtil.getField(AbstractSelectableChannel.class, delegate, "keyLock");
 
-                        Object keyLock = ReflectionUtil.getField(AbstractSelectableChannel.class, sniffyChannel, "keyLock");
-                        Object delegateKeyLock = ReflectionUtil.getField(AbstractSelectableChannel.class, delegate, "keyLock");
+                    // We're always obtaining monitor of wrapper (SniffySelector) first in order to avoid dead locks
 
+                    //noinspection SynchronizationOnLocalVariableOrMethodParameter
+                    synchronized (keyLock) {
                         //noinspection SynchronizationOnLocalVariableOrMethodParameter
                         synchronized (delegateKeyLock) {
-                            //noinspection SynchronizationOnLocalVariableOrMethodParameter
-                            synchronized (keyLock) {
 
-                                SelectionKey[] delegateKeys = ReflectionUtil.getField(AbstractSelectableChannel.class, delegate, "keys");
-                                List<SelectionKey> sniffyKeys = new ArrayList<SelectionKey>(delegateKeys.length);
-                                for (SelectionKey delegateKey : delegateKeys) {
-                                    sniffyKeys.add(null == delegateKey ? null : wrap(delegateKey, this, sniffyChannel));
-                                }
-                                ReflectionUtil.setField(AbstractSelectableChannel.class, sniffyChannel, "keys", sniffyKeys.toArray(new SelectionKey[0]));
+                            SelectionKey[] delegateKeys = ReflectionUtil.getField(AbstractSelectableChannel.class, delegate, "keys");
+                            List<SelectionKey> sniffyKeys = new ArrayList<SelectionKey>(delegateKeys.length);
+                            for (SelectionKey delegateKey : delegateKeys) {
+                                sniffyKeys.add(null == delegateKey ? null : wrap(delegateKey, this, sniffyChannel));
+                            }
+                            ReflectionUtil.setField(AbstractSelectableChannel.class, sniffyChannel, "keys", sniffyKeys.toArray(new SelectionKey[0]));
 
-                                ReflectionUtil.setField(AbstractSelectableChannel.class, delegate, "keyCount"
-                                        , ReflectionUtil.getField(AbstractSelectableChannel.class, sniffyChannel, "keyCount"));
+                            ReflectionUtil.setField(AbstractSelectableChannel.class, delegate, "keyCount"
+                                    ,ReflectionUtil.getField(AbstractSelectableChannel.class, sniffyChannel, "keyCount"));
 
-                            }
                         }
-
-                    } catch (IllegalAccessException e) {
-                        e.printStackTrace();
-                    } catch (NoSuchFieldException e) {
-                        e.printStackTrace();
                     }
+
+                } catch (Exception e) {
+                    e.printStackTrace();
                 }
             }
         }
+
     }
 
     /**