adopt the game theoretically sound 'shrug and let the user get away with it' strategy instead

Author: David R. MacIver

src/com/rabbitmq/client/impl/ChannelManager.java

@@ -126,13 +126,36 @@ private void addChannel(ChannelN chan) {
         _channelMap.put(chan.getChannelNumber(), chan);
     }
 
+    /**
+     * Remove the argument channel from the channel map. 
+     * This method must be safe to call multiple times on the same channel. If 
+     * it is not then things go badly wrong.
+     */
     public synchronized void disconnectChannel(ChannelN channel) {
         int channelNumber = channel.getChannelNumber();
-        if (_channelMap.remove(channelNumber) != channel)
-            throw new IllegalStateException(
-                "We have attempted to "
-                + "create a disconnect a channel that's no longer in "
-                + "use. This should never happen. Please report this as a bug.");
-        channelNumberAllocator.free(channelNumber);
+       
+        // Warning, here be dragons. Not great big ones, but little baby ones
+        // which will nibble on your toes and occasionally trip you up when 
+        // you least expect it. 
+        // Basically, there's a race that can end us up here. It almost never 
+        // happens, but it's easier to repair it when it does than prevent it 
+        // from happening in the first place. 
+        // If we end up doing a Channel.close in one thread and a Channel.open
+        // with the same channel number in another, the two can overlap in such
+        // a way as to cause disconnectChannel on the old channel to try to 
+        // remove the new one. Ideally we would fix this race at the source,
+        // but it's much easier to just catch it here.   
+        synchronized(_channelMap){
+          ChannelN existing = _channelMap.remove(channelNumber);
+          // Nothing to do here. Move along. 
+          if(existing == null) return;
+          // Oops, we've gone and stomped on someone else's channel. Put it back
+          // and pretend we didn't touch it. 
+          else if(existing != channel){
+            _channelMap.put(channelNumber, existing);
+            return;
+          }
+          channelNumberAllocator.free(channelNumber);
+        } 
     }
 }