Skip to content

Commit

Permalink
Refactor GetLine
Browse files Browse the repository at this point in the history 
The class name was a problem, because the method that we want to call on it is
also called GetLine, which looks too much like a constructor in C++, so I
renamed it to LineBuffer.  I haven't yet renamed the child classes.

Also, it was impossible to distinguish between the different causes of
GetLine() returning false (timeout and signal) and EOF could only be checked by
calling a different function. Now all of these causes throw different
exceptions.

This really fixes the problem with attaching a debugger to a process waiting in
GetLine, which would previously cause the read() to return immediately with
EINTR, which we never retried because it looked like a timeout.
  • Loading branch information
@qris
qris committed Sep 28, 2017
1 parent 1dd7cb7 commit 22febb6
Show file tree
Hide file tree
Showing 26 changed files with 525 additions and 261 deletions.
90 changes: 73 additions & 17 deletions bin/bbackupctl/bbackupctl.cpp
View file
Expand Up @@ -173,19 +173,33 @@ int main(int argc, const char *argv[])

// Wait for the configuration summary
std::string configSummary;
if(!getLine.GetLine(configSummary, false, PROTOCOL_DEFAULT_TIMEOUT))
while(true)
{
BOX_ERROR("Failed to receive configuration summary "
"from daemon");
return 1;
}
try
{
configSummary = getLine.GetLine(false, PROTOCOL_DEFAULT_TIMEOUT);
break;
}
catch(BoxException &e)
{
if(EXCEPTION_IS_TYPE(e, CommonException, SignalReceived))
{
// try again
continue;
}
else if(EXCEPTION_IS_TYPE(e, CommonException, GetLineEOF))
{
BOX_ERROR("Server rejected the connection. Are you running "
"bbackupctl as the same user as the daemon?");
}
else
{
BOX_ERROR("Failed to receive configuration summary "
"from daemon: " << e.what());
}

// Was the connection rejected by the server?
if(getLine.IsEOF())
{
BOX_ERROR("Server rejected the connection. Are you running "
"bbackupctl as the same user as the daemon?");
return 1;
return 1;
}
}

// Decode it
Expand All @@ -206,10 +220,28 @@ int main(int argc, const char *argv[])
" MaxUploadWait = " << maxUploadWait << " seconds");

std::string stateLine;
if(!getLine.GetLine(stateLine, false, PROTOCOL_DEFAULT_TIMEOUT) || getLine.IsEOF())
while(true)
{
BOX_ERROR("Failed to receive state line from daemon");
return 1;
try
{
stateLine = getLine.GetLine(false, PROTOCOL_DEFAULT_TIMEOUT);
break;
}
catch(BoxException &e)
{
if(EXCEPTION_IS_TYPE(e, CommonException, SignalReceived))
{
// try again
continue;
}
else
{
BOX_ERROR("Failed to receive configuration summary "
"from daemon: " << e.what());
}

return 1;
}
}

// Decode it
Expand Down Expand Up @@ -297,13 +329,37 @@ int main(int argc, const char *argv[])
}

// Read the response
std::string line;
bool syncIsRunning = false;
bool finished = false;

while(command != NoCommand && !finished && !getLine.IsEOF() &&
getLine.GetLine(line, false, PROTOCOL_DEFAULT_TIMEOUT))
while(command != NoCommand && !finished)
{
std::string line;
try
{
line = getLine.GetLine(false, PROTOCOL_DEFAULT_TIMEOUT);
}
catch(BoxException &e)
{
if(EXCEPTION_IS_TYPE(e, CommonException, SignalReceived))
{
// try again
continue;
}
else if(EXCEPTION_IS_TYPE(e, CommonException, GetLineEOF))
{
BOX_ERROR("Server disconnected unexpectedly. It may have shut "
"down.");
}
else
{
BOX_ERROR("Failed to receive configuration summary "
"from daemon: " << e.what());
}

break;
}

BOX_TRACE("Received line: " << line);

if(line.substr(0, 6) == "state ")
Expand Down
2 changes: 1 addition & 1 deletion bin/bbackupquery/bbackupquery.cpp
View file
Expand Up @@ -467,7 +467,7 @@ int main(int argc, const char *argv[])

try
{
cmd_str = apGetLine->GetLine();
cmd_str = apGetLine->GetLine(false);
}
catch(CommonException &e)
{
Expand Down
107 changes: 67 additions & 40 deletions lib/bbackupd/BackupDaemon.cpp
View file
Expand Up @@ -1850,36 +1850,51 @@ int BackupDaemon::UseScriptToSeeIfSyncAllowed()

// Run it?
pid_t pid = 0;
try
while(true)
{
std::auto_ptr<IOStream> pscript(LocalProcessStream(script,
pid));

// Read in the result
IOStreamGetLine getLine(*pscript);
std::string line;
if(getLine.GetLine(line, true, 30000)) // 30 seconds should be enough
try
{
std::auto_ptr<IOStream> pscript(LocalProcessStream(script, pid));

// Read in and parse the script output:
IOStreamGetLine getLine(*pscript);
std::string line = getLine.GetLine(true, 30000); // 30 seconds should be enough
waitInSeconds = BackupDaemon::ParseSyncAllowScriptOutput(script, line);
}
else
catch(BoxException &e)
{
BOX_ERROR("SyncAllowScript output nothing within "
"30 seconds, waiting 5 minutes to try again"
" (" << script << ")");
if(EXCEPTION_IS_TYPE(e, CommonException, SignalReceived))
{
// try again
continue;
}
else if(EXCEPTION_IS_TYPE(e, CommonException, GetLineEOF))
{
BOX_ERROR("SyncAllowScript exited without any output "
"(" << script << ")");
}
else if(EXCEPTION_IS_TYPE(e, CommonException, IOStreamTimedOut))
{
BOX_ERROR("SyncAllowScript output nothing within "
"30 seconds, waiting 5 minutes to try again "
"(" << script << ")");
waitInSeconds = 5 * 60;
}
else
{
// Ignore any other exceptions, and log that something bad happened.
BOX_ERROR("Unexpected error while trying to execute the "
"SyncAllowScript: " << e.what() << " (" << script << ")");
}
}
}
catch(std::exception &e)
{
BOX_ERROR("Internal error running SyncAllowScript: "
<< e.what() << " (" << script << ")");
}
catch(...)
{
// Ignore any exceptions
// Log that something bad happened
BOX_ERROR("Unknown error running SyncAllowScript (" <<
script << ")");
catch(...)
{
// Ignore any other exceptions, and log that something bad happened.
BOX_ERROR("Unexpected error while trying to execute the SyncAllowScript "
"(" << script << ")");
}

break;
}

// Wait and then cleanup child process, if any
Expand Down Expand Up @@ -2141,17 +2156,15 @@ void BackupDaemon::WaitOnCommandSocket(box_time_t RequiredDelay, bool &DoSyncFla
ASSERT(mapCommandSocketInfo->mapGetLine.get() != 0);

// Ping the remote side, to provide errors which will mean the socket gets closed
mapCommandSocketInfo->mpConnectedSocket->Write("ping\n", 5,
timeout);
mapCommandSocketInfo->mpConnectedSocket->Write("ping\n", 5, timeout);

// Wait for a command or something on the socket
std::string command;
while(mapCommandSocketInfo->mapGetLine.get() != 0
&& !mapCommandSocketInfo->mapGetLine->IsEOF()
&& mapCommandSocketInfo->mapGetLine->GetLine(command, false /* no preprocessing */, timeout))
while(mapCommandSocketInfo->mapGetLine.get() != 0)
{
BOX_TRACE("Receiving command '" << command
<< "' over command socket");
command = mapCommandSocketInfo->mapGetLine->GetLine(
false /* no preprocessing */, timeout);
BOX_TRACE("Received command '" << command << "' over command socket");

bool sendOK = false;
bool sendResponse = true;
Expand Down Expand Up @@ -2201,13 +2214,6 @@ void BackupDaemon::WaitOnCommandSocket(box_time_t RequiredDelay, bool &DoSyncFla
// Set timeout to something very small, so this just checks for data which is waiting
timeout = 1;
}

// Close on EOF?
if(mapCommandSocketInfo->mapGetLine.get() != 0 &&
mapCommandSocketInfo->mapGetLine->IsEOF())
{
CloseCommandConnection();
}
}
catch(ConnectionException &ce)
{
Expand All @@ -2227,10 +2233,30 @@ void BackupDaemon::WaitOnCommandSocket(box_time_t RequiredDelay, bool &DoSyncFla
CloseCommandConnection();
}
}
catch(BoxException &e)
{
if(EXCEPTION_IS_TYPE(e, CommonException, SignalReceived) ||
EXCEPTION_IS_TYPE(e, CommonException, IOStreamTimedOut))
{
// No special handling, wait for us to be called again
return;
}
else if(EXCEPTION_IS_TYPE(e, CommonException, GetLineEOF))
{
// Need to close the connection, then we can just return, and next time
// we're called we'll listen for a new one.
CloseCommandConnection();
return;
}
else
{
BOX_ERROR("Failed to read from command socket: " << e.what());
return;
}
}
catch(std::exception &e)
{
BOX_ERROR("Failed to write to command socket: " <<
e.what());
BOX_ERROR("Failed to write to command socket: " << e.what());

// If an error occurs, and there is a connection active,
// just close that connection and continue. Otherwise,
Expand Down Expand Up @@ -2313,6 +2339,7 @@ void BackupDaemon::SendSyncStartOrFinish(bool SendStart)
mapCommandSocketInfo->mpConnectedSocket.get() != 0)
{
std::string message = SendStart ? "start-sync" : "finish-sync";
BOX_TRACE("Writing to command socket: " << message);
try
{
message += "\n";
Expand Down
82 changes: 52 additions & 30 deletions lib/bbstored/BBStoreDHousekeeping.cpp
View file
Expand Up @@ -214,49 +214,71 @@ bool BackupStoreDaemon::CheckForInterProcessMsg(int AccountNum, int MaximumWaitT
return false;
}

// First, check to see if it's EOF -- this means something has gone wrong, and the housekeeping should terminate.
if(mInterProcessComms.IsEOF())
{
BOX_INFO("Housekeeping process was hungup by main daemon, terminating");
SetTerminateWanted();
return true;
}

// Get a line, and process the message
std::string line;
if(mInterProcessComms.GetLine(line, false /* no pre-processing */, MaximumWaitTime))
{
BOX_TRACE("Housekeeping received command '" << line <<
"' over interprocess comms");

int account = 0;

if(line == "h")
try
{
line = mInterProcessComms.GetLine(false /* no pre-processing */,
MaximumWaitTime);
}
catch(BoxException &e)
{
if(EXCEPTION_IS_TYPE(e, CommonException, SignalReceived) ||
EXCEPTION_IS_TYPE(e, CommonException, IOStreamTimedOut))
{
// HUP signal received by main process
SetReloadConfigWanted();
return true;
// No special handling, just return empty-handed. We will be called again.
return false;
}
else if(line == "t")
else if(EXCEPTION_IS_TYPE(e, CommonException, GetLineEOF))
{
// Terminate signal received by main process
// This means something has gone wrong with the main server process, and the
// housekeeping process should also terminate.
BOX_INFO("Housekeeping process was hungup by main daemon, terminating");
SetTerminateWanted();
return true;
}
else if(sscanf(line.c_str(), "r%x", &account) == 1)
else
{
// Main process is trying to lock an account -- are we processing it?
if(account == AccountNum)
{
// Yes! -- need to stop now so when it retries to get the lock, it will succeed
BOX_INFO("Housekeeping on account " <<
BOX_FORMAT_ACCOUNT(AccountNum) <<
"giving way to client connection");
return true;
}
throw;
}
}

BOX_TRACE("Housekeeping received command '" << line <<
"' over interprocess comms");

int account = 0;

if(line == "h")
{
// HUP signal received by main process
SetReloadConfigWanted();
return true;
}
else if(line == "t")
{
// Terminate signal received by main process
SetTerminateWanted();
return true;
}
else if(sscanf(line.c_str(), "r%x", &account) == 1)
{
// Main process is trying to lock an account -- are we processing it?
if(account == AccountNum)
{
// Yes! -- need to stop now so when it retries to get the lock, it will succeed
BOX_INFO("Housekeeping on account " <<
BOX_FORMAT_ACCOUNT(AccountNum) <<
"giving way to client connection");
return true;
}
}
else
{
THROW_EXCEPTION_MESSAGE(CommonException, Internal, "Housekeeping received "
"unexpected command from main daemon: " << line);
}

return false;
}

Expand Down
1 change: 1 addition & 0 deletions lib/common/CommonException.txt
View file
Expand Up @@ -58,3 +58,4 @@ ReferenceNotFound 50 The database does not contain an expected reference
TimersNotInitialised 51 The timer framework should have been ready at this point
InvalidConfiguration 52 Some required values are missing or incorrect in the configuration file
IOStreamTimedOut 53 A network operation timed out
SignalReceived 54 The function call returned because the process received a signal

0 comments on commit 22febb6

Please sign in to comment.