Handle nullptr return values to InboundLedgers::findCreate

In normal operation, InboundLedgers::findCreate never returns null, but
during system shutdown, it will return null.

Since this only happens in system shutdown, when findCreate returns null
the calling function stops what it was doing and returns.

During testing, an issue where destroying the application object
and creating a new one caused problems with a static PathTable. This table
is now cleared when re-initialized.
This commit is contained in:
Scott Determan
2015-01-07 17:35:40 -05:00
committed by Vinnie Falco
parent 0d0eec6345
commit b0781622b2
4 changed files with 83 additions and 15 deletions

View File

@@ -966,6 +966,14 @@ public:
getApp().getInboundLedgers().findCreate(nextLedger->getParentHash(),
nextLedger->getLedgerSeq() - 1,
InboundLedger::fcHISTORY);
if (!acq)
{
// On system shutdown, findCreate may return a nullptr
WriteLog (lsTRACE, LedgerMaster)
<< "findCreate failed to return an inbound ledger";
return;
}
if (acq->isComplete() && !acq->isFailed())
ledger = acq->getLedger();
else if ((missing > 40000) && getApp().getOPs().shouldFetchPack(missing))
@@ -1117,8 +1125,15 @@ public:
InboundLedger::pointer acq =
getApp().getInboundLedgers ().findCreate (hash, seq, InboundLedger::fcGENERIC);
if (!acq->isDone ())
if (!acq)
{
// On system shutdown, findCreate may return a nullptr
WriteLog (lsTRACE, LedgerMaster)
<< "findCreate failed to return an inbound ledger";
return {};
}
if (!acq->isDone()) {
}
else if (acq->isComplete () && !acq->isFailed ())
{
@@ -1129,6 +1144,15 @@ public:
WriteLog (lsWARNING, LedgerMaster) << "Failed to acquire a published ledger";
getApp().getInboundLedgers().dropLedger(hash);
acq = getApp().getInboundLedgers().findCreate(hash, seq, InboundLedger::fcGENERIC);
if (!acq)
{
// On system shutdown, findCreate may return a nullptr
WriteLog (lsTRACE, LedgerMaster)
<< "findCreate failed to return an inbound ledger";
return {};
}
if (acq->isComplete())
{
if (acq->isFailed())

View File

@@ -40,6 +40,7 @@
#include <google/protobuf/stubs/common.h>
#include <boost/program_options.hpp>
#include <cstdlib>
#include <thread>
#if defined(BEAST_LINUX) || defined(BEAST_MAC) || defined(BEAST_BSD)
#include <sys/resource.h>
@@ -154,9 +155,35 @@ setupConfigForUnitTests (Config* config)
config->importNodeDatabase = beast::StringPairArray ();
}
static
int
runUnitTests(std::string const& pattern, std::string const& argument)
static int runShutdownTests ()
{
// Shutdown tests can not be part of the normal unit tests in 'runUnitTests'
// because it needs to create and destroy an application object.
int const numShutdownIterations = 20;
// Give it enough time to sync and run a bit while synced.
std::chrono::seconds const serverUptimePerIteration (4 * 60);
for (int i = 0; i < numShutdownIterations; ++i)
{
std::cerr << "\n\nStarting server. Iteration: " << i << "\n"
<< std::endl;
std::unique_ptr<Application> app (make_Application (deprecatedLogs()));
auto shutdownApp = [&app](std::chrono::seconds sleepTime, int iteration)
{
std::this_thread::sleep_for (sleepTime);
std::cerr << "\n\nStopping server. Iteration: " << iteration << "\n"
<< std::endl;
app->signalStop();
};
std::thread shutdownThread (shutdownApp, serverUptimePerIteration, i);
setupServer();
startServer();
shutdownThread.join();
}
return EXIT_SUCCESS;
}
static int runUnitTests (std::string const& pattern,
std::string const& argument)
{
// Config needs to be set up before creating Application
setupConfigForUnitTests (&getConfig ());
@@ -208,6 +235,7 @@ int run (int argc, char** argv)
("rpc_ip", po::value <std::string> (), "Specify the IP address for RPC command. Format: <ip-address>[':'<port-number>]")
("rpc_port", po::value <int> (), "Specify the port number for RPC command.")
("standalone,a", "Run with no peers.")
("shutdowntest", po::value <std::string> ()->implicit_value (""), "Perform shutdown tests.")
("unittest,u", po::value <std::string> ()->implicit_value (""), "Perform unit tests.")
("unittest-arg", po::value <std::string> ()->implicit_value (""), "Supplies argument to unit tests.")
("parameters", po::value< vector<string> > (), "Specify comma separated parameters.")
@@ -269,6 +297,7 @@ int run (int argc, char** argv)
&& !vm.count ("parameters")
&& !vm.count ("fg")
&& !vm.count ("standalone")
&& !vm.count ("shutdowntest")
&& !vm.count ("unittest"))
{
std::string logMe = DoSustain (getConfig ().getDebugLogFile ().string());
@@ -384,6 +413,11 @@ int run (int argc, char** argv)
}
}
if (vm.count ("shutdowntest"))
{
return runShutdownTests ();
}
if (iResult == 0)
{
if (!vm.count ("parameters"))

View File

@@ -1185,8 +1185,9 @@ void fillPaths (Pathfinder::PaymentType type, PathCostList const& costs)
void Pathfinder::initPathTable ()
{
// CAUTION: Do not include rules that build default paths
fillPaths(
pt_XRP_to_XRP, {});
mPathTable.clear();
fillPaths (pt_XRP_to_XRP, {});
fillPaths(
pt_XRP_to_nonXRP, {

View File

@@ -76,13 +76,18 @@ Json::Value doLedgerRequest (RPC::Context& context)
{
// We don't have the ledger we need to figure out which ledger
// they want. Try to get it.
Json::Value jvResult
= getApp().getInboundLedgers().findCreate (
refHash, refIndex, InboundLedger::fcGENERIC)
->getJson (0);
jvResult[jss::error] = "ledgerNotFound";
return jvResult;
if (auto il = getApp().getInboundLedgers().findCreate (
refHash, refIndex, InboundLedger::fcGENERIC))
{
Json::Value jvResult = il->getJson (0);
jvResult[jss::error] = "ledgerNotFound";
return jvResult;
}
// findCreate failed to return an inbound ledger. App is likely shutting down
return Json::Value();
}
ledgerHash = ledger->getLedgerHash (ledgerIndex);
@@ -102,9 +107,13 @@ Json::Value doLedgerRequest (RPC::Context& context)
else
{
// Try to get the desired ledger
auto il = getApp().getInboundLedgers().findCreate (
ledgerHash, 0, InboundLedger::fcGENERIC);
return il->getJson (0);
if (auto il = getApp ().getInboundLedgers ().findCreate (
ledgerHash, 0, InboundLedger::fcGENERIC))
{
return il->getJson (0);
}
return RPC::make_error (
rpcNOT_READY, "findCreate failed to return an inbound ledger");
}
}