[1543] Abort on problem requesting a socket

If it is not one of the "known to be OK" exceptions, we abort.

diff --git a/src/lib/server_common/portconfig.cc b/src/lib/server_common/portconfig.cc
index 556f0da75c0f3b282271fe3c38d09475608b416b..62e291cb0784c87cdf1236cd43f6f35aea365481 100644 (file)
--- a/src/lib/server_common/portconfig.cc
+++ b/src/lib/server_common/portconfig.cc
@@ -136,7 +136,7 @@ installListenAddresses(const AddressList& newAddresses,
         setAddresses(service, newAddresses);
         addressStore = newAddresses;
     }
-    catch (const exception& e) {
+    catch (const SocketRequestor::NonFatalSocketError& e) {
         /*
          * If one of the addresses isn't set successfully, we will restore
          * the old addresses, the behavior is that either all address are
@@ -153,7 +153,7 @@ installListenAddresses(const AddressList& newAddresses,
         LOG_ERROR(logger, SRVCOMM_ADDRESS_FAIL).arg(e.what());
         try {
             setAddresses(service, addressStore);
-        } catch (const exception& e2) {
+        } catch (const SocketRequestor::NonFatalSocketError& e2) {
             LOG_FATAL(logger, SRVCOMM_ADDRESS_UNRECOVERABLE).arg(e2.what());
             // If we can't set the new ones, nor the old ones, at least
             // releasing everything should work. If it doesn't, there isn't
@@ -165,6 +165,18 @@ installListenAddresses(const AddressList& newAddresses,
         //manager the new configure doesn't work
         throw;
     }
+    catch (const exception& e) {
+        // Any other kind of exception is fatal. It might mean we are in
+        // inconsistent state with the boss/socket creator, so we abort
+        // to make sure it doesn't last.
+        LOG_FATAL(logger, SRVCOMM_EXCEPTION_ALLOC).arg(e.what());
+        abort();
+    }
+    catch (...) {
+        // As the previous one, but we know even less info
+        LOG_FATAL(logger, SRVCOMM_UNKNOWN_EXCEPTION_ALLOC);
+        abort();
+    }
 }
 
 }

diff --git a/src/lib/server_common/server_common_messages.mes b/src/lib/server_common/server_common_messages.mes
index 0efe38851f6cb2f8700706c53cc33c29f59c7903..eb6362d06a51e1282f14c62ffdfbadd83568b840 100644 (file)
--- a/src/lib/server_common/server_common_messages.mes
+++ b/src/lib/server_common/server_common_messages.mes
@@ -78,6 +78,22 @@ addresses we are going to listen on (eg. there will be one log message
 per pair). This appears only after SRVCOMM_SET_LISTEN, but might
 be hidden, as it has higher debug level.
 
+% SRVCOMM_EXCEPTION_ALLOC exception when allocating a socket: %1
+The process tried to allocate a socket using the socket creator, but an error
+occurred. But it is not one of the errors we are sure are "safe". In this case
+it is unclear if the unsuccessful communication left us and the bind10 process
+in inconsistent state, so we're going to abort to prevent further problems in
+that area.
+
+This is probably a bug in the code, but it could be caused by other unusual
+conditions (like insufficient memory, deleted socket file used for
+communication).
+
+% SRVCOMM_UNKNOWN_EXCEPTION_ALLOC unknown exception when allocating a socket
+The situation is the same as in the SRVCOMM_EXCEPTION_ALLOC case, but we don't
+know further details about the error, because it was signaled by throwing
+something not being an exception. This is definitely a bug.
+
 % SRVCOMM_KEYS_DEINIT deinitializing TSIG keyring
 Debug message indicating that the server is deinitializing the TSIG keyring.
 

diff --git a/src/lib/server_common/tests/portconfig_unittest.cc b/src/lib/server_common/tests/portconfig_unittest.cc
index ee17e56a2391c8199280b900cd0b5017df607cd4..753e808f10190079ddc1c727a99b343a0dca54a8 100644 (file)
--- a/src/lib/server_common/tests/portconfig_unittest.cc
+++ b/src/lib/server_common/tests/portconfig_unittest.cc
@@ -265,7 +265,7 @@ TEST_F(InstallListenAddresses, brokenRollback) {
     sock_requestor_.given_tokens_.clear();
     sock_requestor_.break_rollback_ = true;
     EXPECT_THROW(installListenAddresses(invalid_, store_, dnss_),
-                 SocketRequestor::SocketError);
+                 SocketRequestor::NonFatalSocketError);
     // No addresses here
     EXPECT_TRUE(store_.empty());
     // The first pair should be requested in the first part of the failure to
@@ -295,4 +295,24 @@ TEST_F(InstallListenAddresses, brokenRollback) {
                                 "released");
 }
 
+// Make sure the death tests are filterable away.
+typedef InstallListenAddresses InstallListenAddressesDeathTest;
+
+// We make the socket requestor throw a "fatal" exception, one where we can't be
+// sure the state between processes is consistent. So we abort in that case.
+TEST_F(InstallListenAddressesDeathTest, inconsistent) {
+    AddressList deathAddresses;
+    deathAddresses.push_back(AddressPair("192.0.2.3", 5288));
+    // Make sure it actually kills the application (there should be an abort
+    // in this case)
+    EXPECT_DEATH({
+                    try {
+                        installListenAddresses(deathAddresses, store_, dnss_);
+                    }
+                    // Prevent exceptions killing the application, we need to
+                    // make sure it dies the real hard way
+                    catch (...) {};
+                 }, "");
+}
+
 }

diff --git a/src/lib/testutils/socket_request.h b/src/lib/testutils/socket_request.h
index c06c9696737f03fba89ad5d7b3cdfefa9c4fc722..89279de6f38402cb405c89f8f4013d13cffb05e1 100644 (file)
--- a/src/lib/testutils/socket_request.h
+++ b/src/lib/testutils/socket_request.h
@@ -119,8 +119,9 @@ public:
     /// This creates a new token and fakes a new socket and returns it.
     /// The token is stored.
     ///
-    /// In case the address is 192.0.2.2 or if the break_rollback_ is true
-    /// and address is ::1, it throws.
+    /// In case the address is 192.0.2.2, it throws SocketAllocateError
+    /// or if the break_rollback_ is true and address is ::1, it throws
+    /// ShareError. If the address is 192.0.2.3, it throws SocketError.
     ///
     /// The tokens produced are in form of protocol:address:port:fd. The fds
     /// start at 1 and increase by each successfull call.
@@ -131,13 +132,18 @@ public:
     /// \param mode checked to be DONT_SHARE for now
     /// \param name checked to be dummy_app for now
     /// \return The token and FD
+    /// \throw SocketAllocateError as described above, to test error handling
+    /// \throw ShareError as described above, to test error handling
     /// \throw SocketError as described above, to test error handling
     SocketID requestSocket(Protocol protocol, const std::string& address,
                            uint16_t port, ShareMode mode,
                            const std::string& name)
     {
         if (address == "192.0.2.2") {
-            isc_throw(SocketError, "This address is not allowed");
+            isc_throw(SocketAllocateError, "This address is not allowed");
+        }
+        if (address == "192.0.2.3") {
+            isc_throw(SocketError, "Fatal test error");
         }
         if (address == "::1" && break_rollback_) {
             // This is valid address, but in case we need to break the
@@ -145,7 +151,7 @@ public:
             //
             // We break the second address to see the first one was
             // allocated and then returned
-            isc_throw(SocketError,
+            isc_throw(ShareError,
                       "This address is available, but not for you");
         }
         const std::string proto(protocol == TCP ? "TCP" : "UDP");