Commit b0d52a60 authored by Michal 'vorner' Vaner's avatar Michal 'vorner' Vaner
Browse files

[1543] Abort on problem requesting a socket

If it is not one of the "known to be OK" exceptions, we abort.
parent 058f2664
......@@ -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();
}
}
}
......
......@@ -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.
......
......@@ -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 (...) {};
}, "");
}
}
......@@ -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");
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment