[Jython-checkins] jython: Close sockets more carefully. Fixes #2470, #2471

[jim.baker]

https://hg.python.org/jython/rev/63a2a4f6aa42
changeset:   7908:63a2a4f6aa42
user:        Nick Bailey <nickmbailey at gmail.com>
date:        Tue Feb 23 22:53:40 2016 -0700
summary:
  Close sockets more carefully. Fixes #2470, #2471

Fixes two somewhat related issues with respect to closing sockets:

* Peer closes do not mean the socket is no longer writable, just that
  the peer has closed its half of the channel (half-closed). Fixed by
  always allowing the channel to be written to once half closed (#2470).

* Server sockets now shutdown their thread pools asynchronously with
  their closing, so as to not deadlock (#2471).

files:
  Lib/_socket.py |  52 +++++++++++++++++++------------------
  1 files changed, 27 insertions(+), 25 deletions(-)


diff --git a/Lib/_socket.py b/Lib/_socket.py
--- a/Lib/_socket.py
+++ b/Lib/_socket.py
@@ -729,6 +729,7 @@
         self.selectors = CopyOnWriteArrayList()
         self.options = {}  # deferred options until bootstrap
         self.peer_closed = False
+        self.channel_closed = False
 
         # Reference count this underlying socket
         self.open_lock = Lock()
@@ -875,12 +876,13 @@
         if self.connect_handlers:
             self.channel.pipeline().addLast(self.python_inbound_handler)
         
-        def peer_closed(x):
+        def _peer_closed(x):
             log.debug("Peer closed channel %s", x, extra={"sock": self})
+            self.channel_closed = True
             self.incoming.put(_PEER_CLOSED)
             self._notify_selectors(hangup=True)
 
-        self.channel.closeFuture().addListener(peer_closed)
+        self.channel.closeFuture().addListener(_peer_closed)
 
     def connect(self, addr):
         # Unwrapped sockets can immediately perform the post-connect step
@@ -1054,30 +1056,30 @@
 
             if self.channel is None:
                 return
+            
+            close_future = self.channel.close()
+            if close_future.isSuccess():
+                close_future.addListener(self._finish_closing)
 
-            try:
-                self.channel.close().sync()
-            except RejectedExecutionException:
-                # Do not care about tasks that attempt to schedule after close
-                pass
-            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)
-                self.accepted_children -= 1
-                while True:
-                    child = self.child_queue.poll()
-                    if child is None:
-                        break
-                    log.debug("Closed child socket %s not yet accepted", child, extra={"sock": self})
-                    child.close()
-            else:
-                msgs = []
-                self.incoming.drainTo(msgs)
-                for msg in msgs:
-                    if msg is not _PEER_CLOSED:
-                        msg.release()
+    def _finish_closing(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)
+            self.accepted_children -= 1
+            while True:
+                child = self.child_queue.poll()
+                if child is None:
+                    break
+                log.debug("Closed child socket %s not yet accepted", child, extra={"sock": self})
+                child.close()
+        else:
+            msgs = []
+            self.incoming.drainTo(msgs)
+            for msg in msgs:
+                if msg is not _PEER_CLOSED:
+                    msg.release()
 
-            log.debug("Closed socket", extra={"sock": self})
+        log.debug("Closed socket", extra={"sock": self})
 
     def shutdown(self, how):
         log.debug("Got request to shutdown socket how=%s", how, extra={"sock": self})
@@ -1121,7 +1123,7 @@
         return 0
 
     def _writable(self):
-        return self.channel and self.channel.isActive() and self.channel.isWritable()
+        return self.channel_closed or (self.channel and self.channel.isActive() and self.channel.isWritable())
 
     can_write = _writable
 

-- 
Repository URL: https://hg.python.org/jython