Browse Source

[1454] Exceptions in accept

Michal 'vorner' Vaner 13 years ago
parent
commit
c9a7d521f0
3 changed files with 53 additions and 6 deletions
  1. 12 6
      src/bin/ddns/ddns.py.in
  2. 6 0
      src/bin/ddns/ddns_messages.mes
  3. 35 0
      src/bin/ddns/tests/ddns_test.py

+ 12 - 6
src/bin/ddns/ddns.py.in

@@ -152,12 +152,18 @@ class DDNSServer:
         """
         Accept another connection and create the session receiver.
         """
-        # TODO: Handle some exceptions
-        socket = self._listen_socket.accept()
-        fileno = socket.fileno()
-        logger.debug(TRACE_BASIC, DDNS_NEW_CONN, fileno, socket.getpeername())
-        receiver = isc.util.io.socketsession.SocketSessionReceiver(socket)
-        self._socksession_receivers[fileno] = (socket, receiver)
+        try:
+            sock = self._listen_socket.accept()
+            fileno = sock.fileno()
+            logger.debug(TRACE_BASIC, DDNS_NEW_CONN, fileno,
+                         sock.getpeername())
+            receiver = isc.util.io.socketsession.SocketSessionReceiver(sock)
+            self._socksession_receivers[fileno] = (sock, receiver)
+        except (socket.error, isc.util.io.socketsession.SocketSessionError) \
+            as e:
+            # These exceptions mean the connection didn't work, but we can
+            # continue with the rest
+            logger.error(DDNS_ACCEPT_FAILURE, e)
 
     def handle_request(self, request):
         """

+ 6 - 0
src/bin/ddns/ddns_messages.mes

@@ -19,6 +19,12 @@
 # <topsrcdir>/tools/reorder_message_file.py to make sure the
 # messages are in the correct order.
 
+% DDNS_ACCEPT_FAILURE error accepting a connection: %1
+There was a low-level error when we tried to accept an incoming connection
+(probably coming from b10-auth). We continue serving on whanever other
+connections we already have, but this connection is dropped. The reason
+is logged.
+
 % DDNS_CC_SESSION_ERROR error reading from cc channel: %1
 There was a problem reading from the command and control channel. The
 most likely cause is that the msgq process is not running.

+ 35 - 0
src/bin/ddns/tests/ddns_test.py

@@ -241,6 +241,41 @@ class TestDDNSServer(unittest.TestCase):
         self.assertTrue(isinstance(receiver, FakeSessionReceiver))
         self.assertEqual(socket, receiver.socket())
 
+    def test_accept_fail(self):
+        """
+        Test we don't crash if an accept fails and that we don't modify the
+        internals.
+        """
+        # Make the accept fail
+        def accept_failure():
+            raise socket.error(errno.ECONNABORTED)
+        orig = self.ddns_server._listen_socket.accept
+        self.ddns_server._listen_socket.accept = accept_failure
+        self.assertEqual({}, self.ddns_server._socksession_receivers)
+        # Doesn't raise the exception
+        self.ddns_server.accept()
+        # And nothing is stored
+        self.assertEqual({}, self.ddns_server._socksession_receivers)
+        # Now make the socket receiver fail
+        self.ddns_server._listen_socket.accept = orig
+        def receiver_failure(sock):
+            raise isc.util.io.socketsession.SocketSessionError('Test error')
+        ddns.isc.util.io.socketsession.SocketSessionReceiver = \
+            receiver_failure
+        # Doesn't raise the exception
+        self.ddns_server.accept()
+        # And nothing is stored
+        self.assertEqual({}, self.ddns_server._socksession_receivers)
+        # Check we don't catch everything, so raise just an exception
+        def unexpected_failure(sock):
+            raise Exception('Test error')
+        ddns.isc.util.io.socketsession.SocketSessionReceiver = \
+            unexpected_failure
+        # This one gets through
+        self.assertRaises(Exception, self.ddns_server.accept)
+        # Nothing is stored as well
+        self.assertEqual({}, self.ddns_server._socksession_receivers)
+
     def test_session_called(self):
         """
         Test the run calls handle_session when there's something on the