Issue2471

I think Jython is using netty's channel future incorrectly and causing a deadlock.

The netty docs say it is preferred to add asynchronous listeners rather than calling sync()/await() directly.

https://github.com/netty/netty/blob/4.0/transport/src/main/java/io/netty/channel/ChannelFuture.java#L96

Jython is doing things the incorrect way when closing sockets:

https://github.com/jythontools/jython/blob/master/Lib/_socket.py#L1059

In my twisted application running on jython I can easily cause deadlocks by using apache bench to load test the twisted web server. I think we can safely just add the rest of the shutdown code as a listener on the channel future like so:

diff --git a/Lib/_socket.py b/Lib/_socket.py
index 3b658e5..56d8f4d 100644
--- a/Lib/_socket.py
+++ b/Lib/_socket.py
@@ -1055,11 +1055,10 @@ class _realsocket(object):
             if self.channel is None:
                 return

-            try:
-                self.channel.close().sync()
-            except RejectedExecutionException:
-                # Do not care about tasks that attempt to schedule after close
-                pass
+            closeFuture = self.channel.close()
+            closeFuture.addListener(self.finishClosing)
+
+    def finishClosing(self, _):
         if self.socket_type == SERVER_SOCKET:
             log.debug("Shutting down server socket parent group", extra={"sock": self})
             self.parent_group.shutdownGracefully(0, 100, TimeUnit.MILLISECONDS)


If I apply that patch, I can no longer cause the deadlock.

Looks reasonable, although we should apply a test on the future (isSuccess()) before adding the listener.

Well the promise itself will call the listener immediately if the promise is already done. Is there another reason to check success beforehand?

I was attempting to justify my response to why use isSuccess() when closing this bug out when I realized:

I am wrong, the noise is test_socket is OK, compared to the alternative.

If we don't attempt to add the listener *always*, we simply have a silent race where the thread pool resource may not be cleaned up. And that's far worse.

See https://github.com/netty/netty/issues/2166 - perhaps we can tune the logging.

So latest fix is in https://hg.python.org/jython/rev/08b7720fd321