From 28209bd60a697eb05802dfd25be8b83eaa1355f8 Mon Sep 17 00:00:00 2001 From: Peter Hamilton Date: Wed, 4 Jan 2017 16:02:14 -0500 Subject: [PATCH] Fixes a bug with socket interrupt handling under Python3.5 This change fixes a bug introduced with the addition of Python3.5 support. In Python3.5, SIGINT is silently ignored for system calls (e.g., socket.accept) if the SIGINT signal handler does not raise an exception. This causes the server to delay shutdown when receiving SIGINT until after a new connection has been made. This change updates the server's SIGINT signal handler to raise the correct exception and updates the error handling code while serving connections to account for this change in SIGINT processing. This allows the server to shutdown immediately upon receiving SIGINT. The server unit tests are updated to account for this change. --- kmip/services/server/server.py | 24 ++++++++++++------- .../tests/unit/services/server/test_server.py | 12 ++++++---- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/kmip/services/server/server.py b/kmip/services/server/server.py index a54ebdc..7fec309 100644 --- a/kmip/services/server/server.py +++ b/kmip/services/server/server.py @@ -13,7 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -import errno import logging import logging.handlers as handlers import optparse @@ -287,6 +286,13 @@ class KmipServer(object): def _signal_handler(signal_number, stack_frame): self._is_serving = False + # Python3.5+ silently ignores SIGINT and retries system calls if + # the signal handler does not raise an exception. Explicitly + # detect SIGINT and raise a KeyboardInterrupt exception to regain + # old functionality. + if signal_number == signal.SIGINT: + raise KeyboardInterrupt("SIGINT received") + signal.signal(signal.SIGINT, _signal_handler) signal.signal(signal.SIGTERM, _signal_handler) @@ -296,14 +302,14 @@ class KmipServer(object): try: connection, address = self._socket.accept() except socket.error as e: - if e.errno == errno.EINTR: - self._logger.warning("Interrupting connection service.") - break - else: - self._logger.warning( - "Error detected while establishing new connection." - ) - self._logger.exception(e) + self._logger.warning( + "Error detected while establishing new connection." + ) + self._logger.exception(e) + except KeyboardInterrupt: + self._logger.warning("Interrupting connection service.") + self._is_serving = False + break except Exception as e: self._logger.warning( "Error detected while establishing new connection." diff --git a/kmip/tests/unit/services/server/test_server.py b/kmip/tests/unit/services/server/test_server.py index 7c686c4..cf98641 100644 --- a/kmip/tests/unit/services/server/test_server.py +++ b/kmip/tests/unit/services/server/test_server.py @@ -13,7 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -import errno import logging try: @@ -363,8 +362,7 @@ class TestKmipServer(testtools.TestCase): s._socket = mock.MagicMock() s._setup_connection_handler = mock.MagicMock() - expected_error = socket.error() - expected_error.errno = errno.EINTR + expected_error = KeyboardInterrupt # Test the expected behavior for a normal server/interrupt sequence s._socket.accept = mock.MagicMock( @@ -418,7 +416,13 @@ class TestKmipServer(testtools.TestCase): # Test the signal handler for each expected signal s._is_serving = True handler = signal.getsignal(signal.SIGINT) - handler(None, None) + args = (signal.SIGINT, None) + self.assertRaisesRegex( + KeyboardInterrupt, + "SIGINT received", + handler, + *args + ) self.assertFalse(s._is_serving) s._is_serving = True