Merge pull request #2358 from jamescowens/fix_scraper_deadlock

scraper: Fix scraper deadlock and make activebeforesb dynamic (not require restart)
gridcoin-community · Oct 11, 2021 · 32e7dce · 32e7dce
2 parents aec0a58 + 524e07e
commit 32e7dce
Showing 1 changed file with 27 additions and 20 deletions.
diff --git a/src/gridcoin/scraper/scraper.cpp b/src/gridcoin/scraper/scraper.cpp
@@ -1575,10 +1575,17 @@ void Scraper(bool bSingleShot)
                 }
 
                 sbage = SuperblockAge();
+                int64_t loop_sleep = scraper_sleep();
+
                 _log(logattribute::INFO, "Scraper", "Superblock not needed. age=" + ToString(sbage));
                 _log(logattribute::INFO, "Scraper", "Sleeping for " + ToString(scraper_sleep() / 1000) +" seconds");
 
-                if (!MilliSleep(scraper_sleep())) return;
+                if (!MilliSleep(loop_sleep)) return;
+
+                // To avoid taking a second lock on cs_main here, which happens inside SuperblockAge(), simply add
+                // scraper_sleep() to sbage, since that much time has gone by.
+                sbage += loop_sleep;
+                nBeforeSBSleep = std::max(86400 - active_before_SB() - sbage, (int64_t) 0);
             }
         }
 
@@ -5711,15 +5718,14 @@ Superblock ScraperGetSuperblockContract(bool bStoreConvergedStats, bool bContrac
                         _log(logattribute::INFO, "ScraperGetSuperblockContract", "Stored converged stats.");
                 }
 
-                // I know this involves a copy operation, but it minimizes the lock time on the cache... we may want to
-                // lock before and do a direct assignment, but that will lock the cache for the whole stats computation,
-                // which is not really necessary.
+                Superblock superblock_Prev;
+
                 {
                     LOCK(cs_ConvergedScraperStatsCache);
 
                     ConvergedScraperStatsCache.AddConvergenceToPastConvergencesMap();
 
-                    Superblock superblock_Prev = ConvergedScraperStatsCache.NewFormatSuperblock;
+                    superblock_Prev = ConvergedScraperStatsCache.NewFormatSuperblock;
 
                     ConvergedScraperStatsCache.mScraperConvergedStats = mScraperConvergedStats;
 
@@ -5743,26 +5749,27 @@ Superblock ScraperGetSuperblockContract(bool bStoreConvergedStats, bool bContrac
 
                     // If called from housekeeping, mark bMinHousekeepingComplete true
                     if (bFromHousekeeping) ConvergedScraperStatsCache.bMinHousekeepingComplete = true;
+                }
 
-                    // Signal UI of SBContract status
-                    if (superblock.WellFormed())
+                // Signal UI of SBContract status. cs_ConvergedScraperStatsCache is released before this, because
+                // BitcoinGUI::updateScraperIcon takes a lock on cs_ConvergedScraperStatsCache itself.
+                if (superblock.WellFormed())
+                {
+                    if (superblock_Prev.WellFormed())
                     {
-                        if (superblock_Prev.WellFormed())
-                        {
-                            // If the current is not empty and the previous is not empty and not the same, then there is
-                            // an updated contract.
-                            if (superblock.GetHash() != superblock_Prev.GetHash())
-                                uiInterface.NotifyScraperEvent(scrapereventtypes::SBContract, CT_UPDATED, {});
-                        }
-                        else
-                            // If the previous was empty and the current is not empty, then there is a new contract.
-                            uiInterface.NotifyScraperEvent(scrapereventtypes::SBContract, CT_NEW, {});
+                        // If the current is not empty and the previous is not empty and not the same, then there is
+                        // an updated contract.
+                        if (superblock.GetHash() != superblock_Prev.GetHash())
+                            uiInterface.NotifyScraperEvent(scrapereventtypes::SBContract, CT_UPDATED, {});
                     }
                     else
-                        if (superblock_Prev.WellFormed())
-                            // If the current is empty and the previous was not empty, then the contract has been deleted.
-                            uiInterface.NotifyScraperEvent(scrapereventtypes::SBContract, CT_DELETED, {});
+                        // If the previous was empty and the current is not empty, then there is a new contract.
+                        uiInterface.NotifyScraperEvent(scrapereventtypes::SBContract, CT_NEW, {});
                 }
+                else
+                    if (superblock_Prev.WellFormed())
+                        // If the current is empty and the previous was not empty, then the contract has been deleted.
+                        uiInterface.NotifyScraperEvent(scrapereventtypes::SBContract, CT_DELETED, {});
 
                 _log(logattribute::INFO, "ScraperGetSuperblockContract", "Superblock object generated from convergence");