Merge #1029: [Startup][Refactor][Backport] Memory allocation fail han…

…dler + init step 1 refactored.

eafe9cd [Trivial] Use error() instead of LogPrintf + return false. (furszy)
669794a [Startup][Wallet] Don't continue rescanning the wallet if shutdown was requested. (furszy)
e0e9498 [Logging][Startup]  * Stop loading block indexes on wallet startup if shutdown was requested.  * Wallet loading, wallet rescan and block index load time logged in a more understandable way. (furszy)
ad87023 [Startup][Refactor][Backport] (furszy)

Pull request description:

  * OS memory allocation fail handler.
  * OS signal handler registration method created to remove code duplication.
  * AppInitBasicSetup() method created, better code organization of the wallet initialization setup step (step 1).
  * Stop loading block indexes if wallet shutdown was requested.

ACKs for top commit:
  random-zebra:
    ACK eafe9cd
  Warrows:
    ACK eafe9cd

Tree-SHA512: 65934b760e1b2a0f73ff3aeefefae9fe34b9569d4472dc3d9dd135e11679d1eef8fe0b93b846db844233c28aa23b3e96e36114b7755af3a6237757ddff477fa3

src/init.cpp

@@ -307,14 +307,25 @@ void Shutdown()
  */
 void HandleSIGTERM(int)
 {
- fRequestShutdown = true;
+ StartShutdown();
 }
 
 void HandleSIGHUP(int)
 {
  fReopenDebugLog = true;
 }
 
+#ifndef WIN32
+static void registerSignalHandler(int signal, void(*handler)(int))
+{
+ struct sigaction sa;
+ sa.sa_handler = handler;
+ sigemptyset(&sa.sa_mask);
+ sa.sa_flags = 0;
+ sigaction(signal, &sa, nullptr);
+}
+#endif
+
 bool static InitError(const std::string& str)
 {
  uiInterface.ThreadSafeMessageBox(str, "", CClientUIInterface::MSG_ERROR);
@@ -748,10 +759,20 @@ bool AppInitServers()
  return true;
 }
 
-/** Initialize pivx.
- * @pre Parameters should be parsed and config file should be read.
- */
-bool AppInit2()
+[[noreturn]] static void new_handler_terminate()
+{
+ // Rather than throwing std::bad-alloc if allocation fails, terminate
+ // immediately to (try to) avoid chain corruption.
+ // Since LogPrintf may itself allocate memory, set the handler directly
+ // to terminate first.
+ std::set_new_handler(std::terminate);
+ LogPrintf("Error: Out of memory. Terminating.\n");
+
+ // The log was successful, terminate now.
+ std::terminate();
+};
+
+bool AppInitBasicSetup()
 {
 // ********************************************************* Step 1: setup
 #ifdef _MSC_VER
@@ -764,7 +785,7 @@ bool AppInit2()
  _set_abort_behavior(0, _WRITE_ABORT_MSG | _CALL_REPORTFAULT);
 #endif
 #ifdef WIN32
-// Enable Data Execution Prevention (DEP)
+ // Enable Data Execution Prevention (DEP)
 // Minimum supported OS versions: WinXP SP3, WinVista >= SP1, Win Server 2008
 // A failure is non-critical and needs no further attention!
 #ifndef PROCESS_DEP_ENABLE
@@ -790,26 +811,31 @@ bool AppInit2()
  umask(077);
  }
 
-
- // Clean shutdown on SIGTERM
- struct sigaction sa;
- sa.sa_handler = HandleSIGTERM;
- sigemptyset(&sa.sa_mask);
- sa.sa_flags = 0;
- sigaction(SIGTERM, &sa, NULL);
- sigaction(SIGINT, &sa, NULL);
+ // Clean shutdown on SIGTERMx
+ registerSignalHandler(SIGTERM, HandleSIGTERM);
+ registerSignalHandler(SIGINT, HandleSIGTERM);
 
  // Reopen debug.log on SIGHUP
- struct sigaction sa_hup;
- sa_hup.sa_handler = HandleSIGHUP;
- sigemptyset(&sa_hup.sa_mask);
- sa_hup.sa_flags = 0;
- sigaction(SIGHUP, &sa_hup, NULL);
+ registerSignalHandler(SIGHUP, HandleSIGHUP);
 
  // Ignore SIGPIPE, otherwise it will bring the daemon down if the client closes unexpectedly
  signal(SIGPIPE, SIG_IGN);
 #endif
 
+ std::set_new_handler(new_handler_terminate);
+
+ return true;
+}
+
+/** Initialize pivx.
+ * @pre Parameters should be parsed and config file should be read.
+ */
+bool AppInit2()
+{
+ // ********************************************************* Step 1: setup
+ if (!AppInitBasicSetup())
+ return false;
+
  // ********************************************************* Step 2: parameter interactions
  // Set this early so that parameter interactions go to console
  fPrintToConsole = GetBoolArg("-printtoconsole", false);
@@ -932,7 +958,6 @@ bool AppInit2()
  else if (nScriptCheckThreads > MAX_SCRIPTCHECK_THREADS)
  nScriptCheckThreads = MAX_SCRIPTCHECK_THREADS;
 
- fServer = GetBoolArg("-server", false);
  setvbuf(stdout, NULL, _IOLBF, 0); /// ***TODO*** do we still need this after -printtoconsole is gone?
 
  // Staking needs a CWallet instance, so make sure wallet is enabled
@@ -1088,7 +1113,7 @@ bool AppInit2()
  * that the server is there and will be ready later). Warmup mode will
  * be disabled when initialisation is finished.
  */
- if (fServer) {
+ if (GetBoolArg("-server", false)) {
  uiInterface.InitMessage.connect(SetRPCWarmupStatus);
  if (!AppInitServers())
  return InitError(_("Unable to start HTTP server. See debug log for details."));
@@ -1416,8 +1441,9 @@ bool AppInit2()
 
  uiInterface.InitMessage(_("Loading block index..."));
 
- nStart = GetTimeMillis();
  do {
+ const int64_t load_block_index_start_time = GetTimeMillis();
+
  try {
  UnloadBlockIndex();
  delete pcoinsTip;
@@ -1588,6 +1614,7 @@ bool AppInit2()
 
  fVerifyingBlocks = false;
  fLoaded = true;
+ LogPrintf(" block index %15dms\n", GetTimeMillis() - load_block_index_start_time);
  } while (false);
 
  if (!fLoaded && !ShutdownRequested()) {
@@ -1616,7 +1643,6 @@ bool AppInit2()
  LogPrintf("Shutdown requested. Exiting.\n");
  return false;
  }
- LogPrintf(" block index %15dms\n", GetTimeMillis() - nStart);
 
  boost::filesystem::path est_path = GetDataDir() / FEE_ESTIMATES_FILENAME;
  CAutoFile est_filein(fopen(est_path.string().c_str(), "rb"), SER_DISK, CLIENT_VERSION);
@@ -1652,7 +1678,7 @@ bool AppInit2()
  uiInterface.InitMessage(_("Loading wallet..."));
  fVerifyingBlocks = true;
 
- nStart = GetTimeMillis();
+ const int64_t nWalletStartTime = GetTimeMillis();
  bool fFirstRun = true;
  pwalletMain = new CWallet(strWalletFile);
  DBErrors nLoadWalletRet = pwalletMain->LoadWallet(fFirstRun);
@@ -1700,7 +1726,7 @@ bool AppInit2()
  }
 
  LogPrintf("%s", strErrors.str());
- LogPrintf(" wallet %15dms\n", GetTimeMillis() - nStart);
+ LogPrintf("Wallet completed loading in %15dms\n", GetTimeMillis() - nWalletStartTime);
  zwalletMain = new CzPIVWallet(pwalletMain->strWalletFile);
  pwalletMain->setZWallet(zwalletMain);
 
@@ -1720,9 +1746,11 @@ bool AppInit2()
  if (chainActive.Tip() && chainActive.Tip() != pindexRescan) {
  uiInterface.InitMessage(_("Rescanning..."));
  LogPrintf("Rescanning last %i blocks (from block %i)...\n", chainActive.Height() - pindexRescan->nHeight, pindexRescan->nHeight);
- nStart = GetTimeMillis();
- pwalletMain->ScanForWalletTransactions(pindexRescan, true);
- LogPrintf(" rescan %15dms\n", GetTimeMillis() - nStart);
+ const int64_t nWalletRescanTime = GetTimeMillis();
+ if (pwalletMain->ScanForWalletTransactions(pindexRescan, true, true) == -1) {
+ return error("Shutdown requested over the txs scan. Exiting.");
+ }
+ LogPrintf("Rescan completed in %15dms\n", GetTimeMillis() - nWalletRescanTime);
  pwalletMain->SetBestChain(chainActive.GetLocator());
  nWalletDBUpdated++;
 

src/init.h

@@ -29,6 +29,12 @@ void Shutdown();
 void PrepareShutdown();
 bool AppInit2();
 
+/** Initialize PIVX core: Basic context setup.
+ * @note This can be done before daemonization. Do not call Shutdown() if this function fails.
+ * @pre Parameters should be parsed and config file should be read.
+ */
+bool AppInitBasicSetup();
+
 /** The help message mode determines what help message to show */
 enum HelpMessageMode {
  HMM_BITCOIND,

src/main.cpp

@@ -5293,6 +5293,9 @@ bool static LoadBlockIndexDB(std::string& strError)
  }
  std::sort(vSortedByHeight.begin(), vSortedByHeight.end());
  for (const PAIRTYPE(int, CBlockIndex*) & item : vSortedByHeight) {
+ // Stop if shutdown was requested
+ if (ShutdownRequested()) return false;
+
  CBlockIndex* pindex = item.second;
  pindex->nChainWork = (pindex->pprev ? pindex->pprev->nChainWork : 0) + GetBlockProof(*pindex);
  if (pindex->nStatus & BLOCK_HAVE_DATA) {

src/util.cpp

@@ -114,7 +114,6 @@ bool fDebug = false;
 bool fPrintToConsole = false;
 bool fPrintToDebugLog = true;
 bool fDaemon = false;
-bool fServer = false;
 std::string strMiscWarning;
 bool fLogTimestamps = false;
 bool fLogIPs = false;

src/util.h

@@ -53,7 +53,6 @@ extern std::map<std::string, std::vector<std::string> > mapMultiArgs;
 extern bool fDebug;
 extern bool fPrintToConsole;
 extern bool fPrintToDebugLog;
-extern bool fServer;
 extern std::string strMiscWarning;
 extern bool fLogTimestamps;
 extern bool fLogIPs;

src/wallet/wallet.cpp

@@ -1264,8 +1264,9 @@ bool CWalletTx::WriteToDisk()
  * Scan the block chain (starting in pindexStart) for transactions
  * from or to us. If fUpdate is true, found transactions that already
  * exist in the wallet will be updated.
+ * @returns -1 if process was cancelled or the number of tx added to the wallet.
  */
-int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate)
+int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate, bool fromStartup)
 {
  int ret = 0;
  int64_t nNow = GetTime();
@@ -1290,6 +1291,10 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate)
  if (pindex->nHeight % 100 == 0 && dProgressTip - dProgressStart > 0.0)
  ShowProgress(_("Rescanning..."), std::max(1, std::min(99, (int)((Checkpoints::GuessVerificationProgress(pindex, false) - dProgressStart) / (dProgressTip - dProgressStart) * 100))));
 
+ if (fromStartup && ShutdownRequested()) {
+ return -1;
+ }
+
  CBlock block;
  ReadBlockFromDisk(block, pindex);
  for (CTransaction& tx : block.vtx) {

src/wallet/wallet.h

@@ -506,7 +506,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
  void SyncTransaction(const CTransaction& tx, const CBlock* pblock);
  bool AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, bool fUpdate);
  void EraseFromWallet(const uint256& hash);
- int ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate = false);
+ int ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate = false, bool fromStartup = false);
  void ReacceptWalletTransactions();
  void ResendWalletTransactions();
  CAmount GetBalance() const;