BIND 10 trac1454, updated. fb20b8bb0c4ed9445a2a97504fdfd201ff15df38 [1454] Various minor tweaks based on review
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Jan 19 16:53:51 UTC 2012
The branch, trac1454 has been updated
via fb20b8bb0c4ed9445a2a97504fdfd201ff15df38 (commit)
via 0458ade871c5d0c1dfced7970379965a8b9443c5 (commit)
from 0ee2267ab384059dde33817da15395806f45433f (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 fb20b8bb0c4ed9445a2a97504fdfd201ff15df38
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Thu Jan 19 10:52:34 2012 +0100
[1454] Various minor tweaks based on review
Like renaming variables, rewording comments, improving log messages.
commit 0458ade871c5d0c1dfced7970379965a8b9443c5
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Thu Jan 19 10:52:11 2012 +0100
[1454] Ignore EINTR from select
-----------------------------------------------------------------------
Summary of changes:
src/bin/ddns/ddns.py.in | 53 ++++++++++++---------
src/bin/ddns/ddns_messages.mes | 16 ++++---
src/bin/ddns/tests/ddns_test.py | 99 ++++++++++++++++++++++++++-------------
3 files changed, 107 insertions(+), 61 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/ddns/ddns.py.in b/src/bin/ddns/ddns.py.in
index 088c710..7546ddf 100755
--- a/src/bin/ddns/ddns.py.in
+++ b/src/bin/ddns/ddns.py.in
@@ -25,6 +25,7 @@ from isc.cc import SessionError, SessionTimeout
import isc.util.process
import isc.util.io.socketsession
import select
+import errno
from isc.log_messages.ddns_messages import *
@@ -34,7 +35,7 @@ import signal
isc.log.init("b10-ddns")
logger = isc.log.Logger("ddns")
-DBG_ACTIVITY = logger.DBGLVL_TRACE_BASIC
+TRACE_BASIC = logger.DBGLVL_TRACE_BASIC
DATA_PATH = bind10_config.DATA_PATH
if "B10_FROM_SOURCE" in os.environ:
@@ -88,8 +89,8 @@ class DDNSServer:
self._config_data = self._cc.get_full_config()
self._cc.start()
self._shutdown = False
- # List of the sessions where we get the packets
- self._socket_sessions = {}
+ # List of the session receivers where we get the requests
+ self._socksession_receivers = {}
def config_handler(self, new_config):
'''Update config data.'''
@@ -136,38 +137,38 @@ class DDNSServer:
"""
socket = self._listen_socket.accept()
fileno = socket.fileno()
- logger.debug(DBG_ACTIVITY, DDNS_NEW_CONN, fileno)
- session = isc.util.io.socketsession.SocketSessionReceiver(socket)
- self._socket_sessions[fileno] = (socket, session)
+ logger.debug(TRACE_BASIC, DDNS_NEW_CONN, fileno, socket.getpeername())
+ receiver = isc.util.io.socketsession.SocketSessionReceiver(socket)
+ self._socksession_receivers[fileno] = (socket, receiver)
def handle_request(self, request):
"""
- This is called whenever a new DDNS request comes. Here the magic
- happens, the rest is either subroutines of the magic or the accounting
- around it that calls it from time to time.
+ This is the place where the actual DDNS processing is done. Other
+ methods are either subroutines of this method or methods doing the
+ uninteresting "accounting" stuff, like accepting socket,
+ initialization, etc.
It is called with the request being session as received from
- SocketSessionReceiver, eg. tuple
+ SocketSessionReceiver, i.e. tuple
(socket, local_address, remote_address, data).
"""
# TODO: Implement the magic
pass
- def handle_incoming(self, fileno):
+ def handle_session(self, fileno):
"""
Handle incoming event on the socket with given fileno.
"""
- logger.debug(DBG_ACTIVITY, DDNS_INCOMING, fileno)
- (socket, session) = self._socket_sessions[fileno]
+ logger.debug(TRACE_BASIC, DDNS_SESSION, fileno)
+ (socket, receiver) = self._socksession_receivers[fileno]
try:
- request = session.pop()
- self.handle_request(request)
+ self.handle_request(receiver.pop())
except isc.util.io.socketsession.SocketSessionError as se:
# No matter why this failed, the connection is in unknown, possibly
- # broken state. So, we close the socket and remove the session.
+ # broken state. So, we close the socket and remove the receiver.
logger.warn(DDNS_DROP_CONN, fileno, se)
- del self._socket_sessions[fileno]
socket.close()
+ del self._socksession_receivers[fileno]
def run(self):
'''
@@ -184,17 +185,25 @@ class DDNSServer:
# make such a distinction easily, but once we do, this would
# be the place to catch.
- # TODO: Catch select exceptions, like EINTR
- (reads, writes, exceptions) = \
- select.select([cc_fileno, listen_fileno] +
- list(self._socket_sessions.keys()), [], [])
+ try:
+ (reads, writes, exceptions) = \
+ select.select([cc_fileno, listen_fileno] +
+ list(self._socksession_receivers.keys()), [],
+ [])
+ except select.error as se:
+ # In case it is just interrupted, we continue like nothing
+ # happened
+ if se.args[0] == errno.EINTR:
+ (reads, writes, exceptions) = ([], [], [])
+ else:
+ raise
for fileno in reads:
if fileno == cc_fileno:
self._cc.check_command(True)
elif fileno == listen_fileno:
self.accept()
else:
- self.handle_incoming(fileno)
+ self.handle_session(fileno)
self.shutdown_cleanup()
logger.info(DDNS_STOPPED)
diff --git a/src/bin/ddns/ddns_messages.mes b/src/bin/ddns/ddns_messages.mes
index c68b72f..79c4c49 100644
--- a/src/bin/ddns/ddns_messages.mes
+++ b/src/bin/ddns/ddns_messages.mes
@@ -39,11 +39,6 @@ authoritative server shuts down, the connection would get closed. It also
can mean the system is busy and can't keep up or that the other side got
confused and sent bad data.
-% DDNS_INCOMING incoming activity on file descriptor %1
-A debug message, informing there's some activity on the given file descriptor.
-It will be either a request or the file descriptor will be closed. See
-following log messages to see what of it.
-
% DDNS_MODULECC_SESSION_ERROR error encountered by configuration/command module: %1
There was a problem in the lower level module handling configuration and
control commands. This could happen for various reasons, but the most likely
@@ -51,9 +46,11 @@ cause is that the configuration database contains a syntax error and ddns
failed to start at initialization. A detailed error message from the module
will also be displayed.
-% DDNS_NEW_CONN new connection on file descriptor %1
+% DDNS_NEW_CONN new connection on file descriptor %1 from %2
Debug message. We received a connection and we are going to start handling
-requests from it.
+requests from it. The file descriptor number and the address where the request
+comes from is logged. The connection is over a unix domain socket and is likely
+coming from a b10-auth process.
% DDNS_RECEIVED_SHUTDOWN_COMMAND shutdown command received
The ddns process received a shutdown command from the command channel
@@ -63,6 +60,11 @@ and will now shut down.
The ddns process has successfully started and is now ready to receive commands
and updates.
+% DDNS_SESSION session arrived on file descriptor %1
+A debug message, informing there's some activity on the given file descriptor.
+It will be either a request or the file descriptor will be closed. See
+following log messages to see what of it.
+
% DDNS_SHUTDOWN ddns server shutting down
The ddns process is shutting down. It will no longer listen for new commands
or updates. Any command or update that is being addressed at this moment will
diff --git a/src/bin/ddns/tests/ddns_test.py b/src/bin/ddns/tests/ddns_test.py
index e7202b6..6dd05e3 100755
--- a/src/bin/ddns/tests/ddns_test.py
+++ b/src/bin/ddns/tests/ddns_test.py
@@ -20,20 +20,23 @@ import isc
import ddns
import isc.config
import select
+import errno
import isc.util.io.socketsession
class FakeSocket:
"""
- A fake socket. It only provides a file number.
+ A fake socket. It only provides a file number, peer name and accept method.
"""
def __init__(self, fileno):
self.__fileno = fileno
def fileno(self):
return self.__fileno
+ def getpeername(self):
+ return "fake_unix_socket"
def accept(self):
return FakeSocket(self.__fileno + 1)
-class FakeSession:
+class FakeSessionReceiver:
"""
A fake socket session receiver, for our tests.
"""
@@ -95,10 +98,11 @@ class TestDDNSServer(unittest.TestCase):
cc_session = MyCCSession()
self.assertFalse(cc_session._started)
self.ddns_server = ddns.DDNSServer(cc_session)
- self.cc_session = cc_session
+ self.__cc_session = cc_session
self.assertTrue(cc_session._started)
self.__select_expected = None
self.__select_answer = None
+ self.__select_exception = None
self.__hook_called = False
self.ddns_server._listen_socket = FakeSocket(2)
ddns.select.select = self.__select
@@ -140,9 +144,16 @@ class TestDDNSServer(unittest.TestCase):
"""
A fake select. It checks it was called with the correct parameters and
returns a preset answer.
+
+ If there's an exception stored in __select_exception, it is raised
+ instead and the exception is cleared.
"""
self.assertEqual(self.__select_expected, (reads, writes, exceptions,
timeout))
+ if self.__select_exception is not None:
+ (self.__select_exception, exception) = (None,
+ self.__select_exception)
+ raise exception
answer = self.__select_answer
self.__select_answer = None
self.ddns_server._shutdown = True
@@ -175,7 +186,7 @@ class TestDDNSServer(unittest.TestCase):
Test the check_command is called when there's something on the
socket.
"""
- self.cc_session.check_command = self.__hook
+ self.__cc_session.check_command = self.__hook
self.__select_expected = ([1, 2], [], [], None)
self.__select_answer = ([1], [], [])
self.ddns_server.run()
@@ -191,25 +202,30 @@ class TestDDNSServer(unittest.TestCase):
Test that we can accept a new connection.
"""
# There's nothing before the accept
- ddns.isc.util.io.socketsession.SocketSessionReceiver = FakeSession
- self.assertEqual({}, self.ddns_server._socket_sessions)
+ ddns.isc.util.io.socketsession.SocketSessionReceiver = \
+ FakeSessionReceiver
+ self.assertEqual({}, self.ddns_server._socksession_receivers)
self.ddns_server.accept()
# Now the new socket session receiver is stored in the dict
- self.assertEqual([3], list(self.ddns_server._socket_sessions.keys()))
- (socket, session) = self.ddns_server._socket_sessions[3]
+ # The 3 comes from _listen_socket.accept() - _listen_socket has
+ # fileno 2 and accept returns socket with fileno increased by one.
+ self.assertEqual([3],
+ list(self.ddns_server._socksession_receivers.keys()))
+ (socket, receiver) = self.ddns_server._socksession_receivers[3]
self.assertTrue(isinstance(socket, FakeSocket))
self.assertEqual(3, socket.fileno())
- self.assertTrue(isinstance(session, FakeSession))
- self.assertEqual(socket, session.socket())
+ self.assertTrue(isinstance(receiver, FakeSessionReceiver))
+ self.assertEqual(socket, receiver.socket())
def test_incoming_called(self):
"""
- Test the run calls handle_incoming when there's something on the
+ Test the run calls handle_session when there's something on the
socket.
"""
socket = FakeSocket(3)
- self.ddns_server._socket_sessions = {3: (socket, FakeSession(socket))}
- self.ddns_server.handle_incoming = self.__hook
+ self.ddns_server._socksession_receivers = \
+ {3: (socket, FakeSessionReceiver(socket))}
+ self.ddns_server.handle_session = self.__hook
self.__select_expected = ([1, 2, 3], [], [], None)
self.__select_answer = ([3], [], [])
self.ddns_server.run()
@@ -217,50 +233,69 @@ class TestDDNSServer(unittest.TestCase):
self.assertIsNone(self.__select_answer)
self.assertEqual(3, self.__hook_called)
- def test_handle_incoming_ok(self):
+ def test_handle_session_ok(self):
"""
- Test the handle_incoming pops the session and calls handle_request
+ Test the handle_session pops the receiver and calls handle_request
when everything is OK.
"""
socket = FakeSocket(3)
- session = FakeSession(socket)
+ receiver = FakeSessionReceiver(socket)
# It doesn't really matter what data we use here, it is only passed
# through the code
param = (FakeSocket(4), ('127.0.0.1', 1234), ('127.0.0.1', 1235),
'Some data')
def pop():
return param
- # Prepare data into the session
- session.pop = pop
- self.ddns_server._socket_sessions = {3: (socket, session)}
+ # Prepare data into the receiver
+ receiver.pop = pop
+ self.ddns_server._socksession_receivers = {3: (socket, receiver)}
self.ddns_server.handle_request = self.__hook
# Call it
- self.ddns_server.handle_incoming(3)
+ self.ddns_server.handle_session(3)
# The popped data are passed into the handle_request
self.assertEqual(param, self.__hook_called)
- # The sessions are kept the same
- self.assertEqual({3: (socket, session)},
- self.ddns_server._socket_sessions)
+ # The receivers are kept the same
+ self.assertEqual({3: (socket, receiver)},
+ self.ddns_server._socksession_receivers)
- def test_handle_incoming_fail(self):
+ def test_handle_session_fail(self):
"""
- Test the handle_incoming removes (and closes) the socket and session
- when the session complains.
+ Test the handle_session removes (and closes) the socket and receiver
+ when the receiver complains.
"""
socket = FakeSocket(3)
- session = FakeSession(socket)
+ receiver = FakeSessionReceiver(socket)
def pop():
raise isc.util.io.socketsession.SocketSessionError('Test error')
- session.pop = pop
+ receiver.pop = pop
socket.close = self.__hook
self.__hook_called = False
- self.ddns_server._socket_sessions = {3: (socket, session)}
- self.ddns_server.handle_incoming(3)
- # The "dead" session is removed
- self.assertEqual({}, self.ddns_server._socket_sessions)
+ self.ddns_server._socksession_receivers = {3: (socket, receiver)}
+ self.ddns_server.handle_session(3)
+ # The "dead" receiver is removed
+ self.assertEqual({}, self.ddns_server._socksession_receivers)
# Close is called with no parameter, so the default None
self.assertIsNone(self.__hook_called)
+ def test_select_exception_ignored(self):
+ """
+ Test that the EINTR is ignored in select.
+ """
+ # Prepare the EINTR exception
+ self.__select_exception = select.error(errno.EINTR)
+ # We reuse the test here, as it should act the same. The exception
+ # should just get ignored.
+ self.test_check_command_called()
+
+ def test_select_exception_fatal(self):
+ """
+ Test that other exceptions are fatal to the run.
+ """
+ # Prepare a different exception
+ self.__select_exception = select.error(errno.EBADF)
+ self.__select_expected = ([1, 2], [], [], None)
+ self.assertRaises(select.error, self.ddns_server.run)
+
class TestMain(unittest.TestCase):
def setUp(self):
self._server = MyDDNSServer()
More information about the bind10-changes
mailing list