BIND 10 trac2617, updated. 878c361337c79e35816bb7df20595b8a5faa3491 [2617] treat ECONNRESET on recv as error unconditionally with detailed notes

[BIND 10 source code commits]

The branch, trac2617 has been updated
       via  878c361337c79e35816bb7df20595b8a5faa3491 (commit)
      from  266bfcd67adafe326c13c6d835f5875628972d9f (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 878c361337c79e35816bb7df20595b8a5faa3491
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Thu Jan 31 10:49:57 2013 -0800

    [2617] treat ECONNRESET on recv as error unconditionally with detailed notes

-----------------------------------------------------------------------

Summary of changes:
 src/bin/msgq/msgq.py.in         |   11 +++++------
 src/bin/msgq/msgq_messages.mes  |   16 ++++++++++++++--
 src/bin/msgq/tests/msgq_test.py |    8 ++------
 3 files changed, 21 insertions(+), 14 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/msgq/msgq.py.in b/src/bin/msgq/msgq.py.in
index 9bf0b85..347bf74 100755
--- a/src/bin/msgq/msgq.py.in
+++ b/src/bin/msgq/msgq.py.in
@@ -364,13 +364,12 @@ class MsgQ:
             try:
                 data = sock.recv(length - len(received))
 
-            # If the remote client has closed the socket there seems to be
-            # two possible cases: getting ECONNRESET or receiving empty data.
-            # These cases are possible in normal operation, so we report them
-            # using MsgQCloseOnReceive.
             except socket.error as err:
-                if err.errno == errno.ECONNRESET:
-                    raise MsgQCloseOnReceive(str(err), continued)
+                # This case includes ECONNRESET, which seems to happen when
+                # the remote client has closed its socket at some subtle
+                # timing (it should normally result in receiving empty data).
+                # Since we didn't figure out how exactly that could happen,
+                # we treat it just like other really-unexpected socket errors.
                 raise MsgQReceiveError(str(err))
             if len(data) == 0:
                 raise MsgQCloseOnReceive("EOF", continued)
diff --git a/src/bin/msgq/msgq_messages.mes b/src/bin/msgq/msgq_messages.mes
index ad5c326..7fb962d 100644
--- a/src/bin/msgq/msgq_messages.mes
+++ b/src/bin/msgq/msgq_messages.mes
@@ -99,10 +99,22 @@ the authors when figuring the problem out.
 % MSGQ_RECV_ERR Error reading from socket %1: %2
 There was a low-level error when reading from a socket. The error is logged and
 the corresponding socket is dropped.  The errors include receiving
-broken or (non empty but) incomplete data.  In either case it suggests
+broken or (non empty but) incomplete data.  In either case it usually suggests
 something unexpected happens within the BIND 10 system; it's probably
 better to restart the system, and if it continues it should be
-reported as a bug.
+reported as a bug.  One known, probably non critical case is
+the "connection reset by peer" (or its variants) socket error appearing
+on shutdown.  It's known this happens when the remote client closes the
+connection as part of shutdown process.  Such cases are normally expected
+to be reported as receiving empty data (which we log it at the debug level
+as the MSGQ_CLOSE_ON_RECV message), but for some (yet) unknown reason
+it can also be reported as the system error.  At shutdown time it's expected
+that connections are closed, so it's probably safe to ignore these messages
+in such a case.  We still log them as an error as we've not figured out
+how exactly that can happen.  In future, we may make the shutdown process
+more robust so the msgq daemon can explicitly know when a client shuts down
+more reliably.  If and when it's implemented this error message won't appear
+on shutdown unless there's really something unexpected.
 
 % MSGQ_RECV_HDR Received header: %1
 Debug message. This message includes the whole routing header of a packet.
diff --git a/src/bin/msgq/tests/msgq_test.py b/src/bin/msgq/tests/msgq_test.py
index bcd9bc7..03e0f7b 100644
--- a/src/bin/msgq/tests/msgq_test.py
+++ b/src/bin/msgq/tests/msgq_test.py
@@ -705,18 +705,14 @@ class SocketTests(unittest.TestCase):
         expected_debugs = 0
 
         # if socket.recv() fails due to socket.error, it will be logged
-        # as error or debug depending on the errno.  In either case the socket
-        # will be killed.
+        # as error and the socket will be killed regardless of errno.
         for eno in [errno.ENOBUFS, errno.ECONNRESET]:
             self.__sock_error.errno = eno
             self.__sock.recv_result = self.__sock_error
             self.__killed_socket = None # clear any previuos value
             self.__msgq.process_packet(42, self.__sock)
             self.assertEqual((42, self.__sock), self.__killed_socket)
-            if eno == errno.ENOBUFS:
-                expected_errors += 1
-            else:
-                expected_debugs += 1
+            expected_errors += 1
             self.assertEqual(expected_errors, self.__logger.error_called)
             self.assertEqual(expected_debugs, self.__logger.debug_called)