Make sure we wait for all scheduled tasks to complete before throwing exceptions further (#662)

Bugfix: We must not throw exceptions to calling function until all async work we spawned has returned

1 files changed, 88 insertions, 48 deletions

diff --git a/src/zenstore/gc.cpp b/src/zenstore/gc.cpp
index 8c02e82c5..f6469c51d 100644
--- a/src/zenstore/gc.cpp
+++ b/src/zenstore/gc.cpp
@@ -633,7 +633,7 @@ GcManager::CollectGarbage(const GcSettings& Settings)
 			{
 				// First remove any cache keys that may own references
 				SCOPED_TIMER(Result.RemoveExpiredDataMS = std::chrono::milliseconds(Timer.GetElapsedTimeMs()); if (Ctx.Settings.Verbose) {
-					ZEN_INFO("GCV2: Removed epxired data for {} referenceners in {}",
+					ZEN_INFO("GCV2: Removed expired data for {} referenceners in {}",
 							 m_GcReferencers.size(),
 							 NiceTimeSpanMs(Result.RemoveExpiredDataMS.count()));
 				});
@@ -648,16 +648,24 @@ GcManager::CollectGarbage(const GcSettings& Settings)
 					GcReferencer*							   Owner = m_GcReferencers[Index];
 					std::pair<std::string, GcReferencerStats>* Stats = &Result.ReferencerStats[Index];
 					WorkLeft.AddCount(1);
-					ThreadPool.ScheduleWork([&Ctx, &WorkLeft, Owner, Stats, &StoreCompactorsLock, &StoreCompactors]() {
-						auto _		 = MakeGuard([&WorkLeft]() { WorkLeft.CountDown(); });
-						Stats->first = Owner->GetGcName(Ctx);
-						SCOPED_TIMER(Stats->second.RemoveExpiredDataStats.ElapsedMS = std::chrono::milliseconds(Timer.GetElapsedTimeMs()););
-						std::unique_ptr<GcStoreCompactor> StoreCompactor(
-							Owner->RemoveExpiredData(Ctx, Stats->second.RemoveExpiredDataStats));
-						if (StoreCompactor)
+					ThreadPool.ScheduleWork([this, &Ctx, &WorkLeft, Owner, Stats, &StoreCompactorsLock, &StoreCompactors]() {
+						auto _ = MakeGuard([&WorkLeft]() { WorkLeft.CountDown(); });
+						try
 						{
-							RwLock::ExclusiveLockScope __(StoreCompactorsLock);
-							StoreCompactors.insert_or_assign(std::move(StoreCompactor), &Stats->second.CompactStoreStats);
+							Stats->first = Owner->GetGcName(Ctx);
+							SCOPED_TIMER(Stats->second.RemoveExpiredDataStats.ElapsedMS =
+											 std::chrono::milliseconds(Timer.GetElapsedTimeMs()););
+							std::unique_ptr<GcStoreCompactor> StoreCompactor(
+								Owner->RemoveExpiredData(Ctx, Stats->second.RemoveExpiredDataStats));
+							if (StoreCompactor)
+							{
+								RwLock::ExclusiveLockScope __(StoreCompactorsLock);
+								StoreCompactors.insert_or_assign(std::move(StoreCompactor), &Stats->second.CompactStoreStats);
+							}
+						}
+						catch (std::exception& Ex)
+						{
+							ZEN_ERROR("GCV2: Failed removing expired data for {}. Reason: '{}'", Owner->GetGcName(Ctx), Ex.what());
 						}
 					});
 				}
@@ -706,21 +714,30 @@ GcManager::CollectGarbage(const GcSettings& Settings)
 						std::pair<std::string, GcReferenceStoreStats>* Stats		  = &Result.ReferenceStoreStats[Index];
 						WorkLeft.AddCount(1);
 						ThreadPool.ScheduleWork(
-							[&Ctx, ReferenceStore, Stats, Index, &WorkLeft, &ReferencePrunersLock, &ReferencePruners]() {
-								auto _		 = MakeGuard([&WorkLeft]() { WorkLeft.CountDown(); });
-								Stats->first = ReferenceStore->GetGcName(Ctx);
-								std::unique_ptr<GcReferencePruner> ReferencePruner;
+							[this, &Ctx, ReferenceStore, Stats, Index, &WorkLeft, &ReferencePrunersLock, &ReferencePruners]() {
+								auto _ = MakeGuard([&WorkLeft]() { WorkLeft.CountDown(); });
+								try
 								{
-									SCOPED_TIMER(Stats->second.CreateReferencePrunersMS =
-													 std::chrono::milliseconds(Timer.GetElapsedTimeMs()););
-									// The ReferenceStore will pick a list of CId entries to check, returning a collector
-									ReferencePruner =
-										std::unique_ptr<GcReferencePruner>(ReferenceStore->CreateReferencePruner(Ctx, Stats->second));
+									Stats->first = ReferenceStore->GetGcName(Ctx);
+									std::unique_ptr<GcReferencePruner> ReferencePruner;
+									{
+										SCOPED_TIMER(Stats->second.CreateReferencePrunersMS =
+														 std::chrono::milliseconds(Timer.GetElapsedTimeMs()););
+										// The ReferenceStore will pick a list of CId entries to check, returning a collector
+										ReferencePruner =
+											std::unique_ptr<GcReferencePruner>(ReferenceStore->CreateReferencePruner(Ctx, Stats->second));
+									}
+									if (ReferencePruner)
+									{
+										RwLock::ExclusiveLockScope __(ReferencePrunersLock);
+										ReferencePruners.insert_or_assign(Index, std::move(ReferencePruner));
+									}
 								}
-								if (ReferencePruner)
+								catch (std::exception& Ex)
 								{
-									RwLock::ExclusiveLockScope __(ReferencePrunersLock);
-									ReferencePruners.insert_or_assign(Index, std::move(ReferencePruner));
+									ZEN_ERROR("GCV2: Failed creating reference pruners for {}. Reason: '{}'",
+											  ReferenceStore->GetGcName(Ctx),
+											  Ex.what());
 								}
 							});
 					}
@@ -767,18 +784,18 @@ GcManager::CollectGarbage(const GcSettings& Settings)
 							std::pair<std::string, GcReferencerStats>* Stats	  = &Result.ReferencerStats[Index];
 							WorkLeft.AddCount(1);
 							ThreadPool.ScheduleWork(
-								[&Ctx, &WorkLeft, Referencer, Index, Stats, &ReferenceCheckersLock, &ReferenceCheckers]() {
+								[this, &Ctx, &WorkLeft, Referencer, Index, Stats, &ReferenceCheckersLock, &ReferenceCheckers]() {
 									auto _ = MakeGuard([&WorkLeft]() { WorkLeft.CountDown(); });
-									// The Referencer will create a reference checker that guarrantees that the references do not change as
+									// The Referencer will create a reference checker that guarantees that the references do not change as
 									// long as it lives
 									std::vector<GcReferenceChecker*> Checkers;
-									{
-										SCOPED_TIMER(Stats->second.CreateReferenceCheckersMS =
-														 std::chrono::milliseconds(Timer.GetElapsedTimeMs()););
-										Checkers = Referencer->CreateReferenceCheckers(Ctx);
-									}
 									try
 									{
+										{
+											SCOPED_TIMER(Stats->second.CreateReferenceCheckersMS =
+															 std::chrono::milliseconds(Timer.GetElapsedTimeMs()););
+											Checkers = Referencer->CreateReferenceCheckers(Ctx);
+										}
 										if (!Checkers.empty())
 										{
 											RwLock::ExclusiveLockScope __(ReferenceCheckersLock);
@@ -789,14 +806,16 @@ GcManager::CollectGarbage(const GcSettings& Settings)
 											}
 										}
 									}
-									catch (std::exception&)
+									catch (std::exception& Ex)
 									{
+										ZEN_ERROR("GCV2: Failed creating reference checkers for {}. Reason: '{}'",
+												  Referencer->GetGcName(Ctx),
+												  Ex.what());
 										while (!Checkers.empty())
 										{
 											delete Checkers.back();
 											Checkers.pop_back();
 										}
-										throw;
 									}
 								});
 						}
@@ -837,10 +856,17 @@ GcManager::CollectGarbage(const GcSettings& Settings)
 								size_t									   Index   = It.second;
 								std::pair<std::string, GcReferencerStats>* Stats   = &Result.ReferencerStats[Index];
 								WorkLeft.AddCount(1);
-								ThreadPool.ScheduleWork([&Ctx, Checker, Index, Stats, &WorkLeft]() {
+								ThreadPool.ScheduleWork([this, &Ctx, Checker, Index, Stats, &WorkLeft]() {
 									auto _ = MakeGuard([&WorkLeft]() { WorkLeft.CountDown(); });
-									SCOPED_TIMER(Stats->second.PreCacheStateMS = std::chrono::milliseconds(Timer.GetElapsedTimeMs()););
-									Checker->PreCache(Ctx);
+									try
+									{
+										SCOPED_TIMER(Stats->second.PreCacheStateMS = std::chrono::milliseconds(Timer.GetElapsedTimeMs()););
+										Checker->PreCache(Ctx);
+									}
+									catch (std::exception& Ex)
+									{
+										ZEN_ERROR("GCV2: Failed precaching for {}. Reason: '{}'", Checker->GetGcName(Ctx), Ex.what());
+									}
 								});
 							}
 							WorkLeft.CountDown();
@@ -886,10 +912,17 @@ GcManager::CollectGarbage(const GcSettings& Settings)
 								size_t									   Index   = It.second;
 								std::pair<std::string, GcReferencerStats>* Stats   = &Result.ReferencerStats[Index];
 								WorkLeft.AddCount(1);
-								ThreadPool.ScheduleWork([&Ctx, Checker, Index, Stats, &WorkLeft]() {
+								ThreadPool.ScheduleWork([this, &Ctx, Checker, Index, Stats, &WorkLeft]() {
 									auto _ = MakeGuard([&WorkLeft]() { WorkLeft.CountDown(); });
-									SCOPED_TIMER(Stats->second.LockStateMS = std::chrono::milliseconds(Timer.GetElapsedTimeMs()););
-									Checker->LockState(Ctx);
+									try
+									{
+										SCOPED_TIMER(Stats->second.LockStateMS = std::chrono::milliseconds(Timer.GetElapsedTimeMs()););
+										Checker->LockState(Ctx);
+									}
+									catch (std::exception& Ex)
+									{
+										ZEN_ERROR("GCV2: Failed locking state for {}. Reason: '{}'", Checker->GetGcName(Ctx), Ex.what());
+									}
 								});
 							}
 							WorkLeft.CountDown();
@@ -943,23 +976,30 @@ GcManager::CollectGarbage(const GcSettings& Settings)
 								GcReferenceStoreStats* Stats  = &Result.ReferenceStoreStats[Index].second;
 								WorkLeft.AddCount(1);
 								ThreadPool.ScheduleWork(
-									[&Ctx, Pruner, Stats, &WorkLeft, &GetUnusedReferences, &StoreCompactorsLock, &StoreCompactors]() {
+									[this, &Ctx, Pruner, Stats, &WorkLeft, &GetUnusedReferences, &StoreCompactorsLock, &StoreCompactors]() {
 										auto _ = MakeGuard([&WorkLeft]() { WorkLeft.CountDown(); });
 										// Go through all the ReferenceCheckers to see if the list of Cids the collector selected are
 										// referenced or not.
-										std::unique_ptr<GcStoreCompactor> StoreCompactor;
+										try
 										{
-											SCOPED_TIMER(Stats->RemoveUnreferencedDataStats.ElapsedMS =
-															 std::chrono::milliseconds(Timer.GetElapsedTimeMs()););
-											StoreCompactor = std::unique_ptr<GcStoreCompactor>(
-												Pruner->RemoveUnreferencedData(Ctx,
-																			   Stats->RemoveUnreferencedDataStats,
-																			   GetUnusedReferences));
+											std::unique_ptr<GcStoreCompactor> StoreCompactor;
+											{
+												SCOPED_TIMER(Stats->RemoveUnreferencedDataStats.ElapsedMS =
+																 std::chrono::milliseconds(Timer.GetElapsedTimeMs()););
+												StoreCompactor = std::unique_ptr<GcStoreCompactor>(
+													Pruner->RemoveUnreferencedData(Ctx,
+																				   Stats->RemoveUnreferencedDataStats,
+																				   GetUnusedReferences));
+											}
+											if (StoreCompactor)
+											{
+												RwLock::ExclusiveLockScope __(StoreCompactorsLock);
+												StoreCompactors.insert_or_assign(std::move(StoreCompactor), &Stats->CompactStoreStats);
+											}
 										}
-										if (StoreCompactor)
+										catch (std::exception& Ex)
 										{
-											RwLock::ExclusiveLockScope __(StoreCompactorsLock);
-											StoreCompactors.insert_or_assign(std::move(StoreCompactor), &Stats->CompactStoreStats);
+											ZEN_ERROR("GCV2: Failed locking state for {}. Reason: '{}'", Pruner->GetGcName(Ctx), Ex.what());
 										}
 									});
 							}