Keep object locked until events are dispatched.

Author: w1ll-i-code

lib/icinga/checkable-check.cpp

@@ -98,10 +98,8 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
 {
 	using Result = Checkable::ProcessingResult;
 
-	{
-		ObjectLock olock(this);
-		m_CheckRunning = false;
-	}
+	ObjectLock olock(this);
+	m_CheckRunning = false;
 
 	if (!cr)
 		return Result::NoCheckResult;
@@ -154,8 +152,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
 	bool reachable = IsReachable();
 	bool notification_reachable = IsReachable(DependencyNotification);
 
-	ObjectLock olock(this);
-
 	CheckResult::Ptr old_cr = GetLastCheckResult();
 	ServiceState old_state = GetStateRaw();
 	StateType old_stateType = GetStateType();
@@ -318,8 +314,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
 	if (is_volatile && IsStateOK(old_state) && IsStateOK(new_state))
 		send_notification = false; /* Don't send notifications for volatile OK -> OK changes. */
 
-	olock.Unlock();
-
 	if (remove_acknowledgement_comments)
 		RemoveAckComments(String(), cr->GetExecutionEnd());
 
@@ -335,25 +329,14 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
 
 	cr->SetVarsAfter(vars_after);
 
-	olock.Lock();
-
+	bool problem_change = false;
 	if (service) {
 		SetLastCheckResult(cr);
 	} else {
 		bool wasProblem = GetProblem();
-
 		SetLastCheckResult(cr);
-
-		if (GetProblem() != wasProblem) {
-			auto services = host->GetServices();
-			olock.Unlock();
-			for (auto& service : services) {
-				Service::OnHostProblemChanged(service, cr, origin);
-			}
-			olock.Lock();
-		}
+		problem_change = GetProblem() != wasProblem;
 	}
-
 	bool was_flapping = IsFlapping();
 
 	UpdateFlappingStatus(cr->GetState());
@@ -384,8 +367,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
 		}
 	}
 
-	olock.Unlock();
-
 #ifdef I2_DEBUG /* I2_DEBUG */
 	Log(LogDebug, "Checkable")
 		<< "Flapping: Checkable " << GetName()
@@ -396,35 +377,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
 		<< "% current: " << GetFlappingCurrent() << "%.";
 #endif /* I2_DEBUG */
 
-	if (recovery) {
-		for (auto& child : children) {
-			if (child->GetProblem() && child->GetEnableActiveChecks()) {
-				auto nextCheck (now + Utility::Random() % 60);
-
-				ObjectLock oLock (child);
-
-				if (nextCheck < child->GetNextCheck()) {
-					child->SetNextCheck(nextCheck);
-				}
-			}
-		}
-	}
-
-	if (stateChange) {
-		/* reschedule direct parents */
-		for (const Checkable::Ptr& parent : GetParents()) {
-			if (parent.get() == this)
-				continue;
-
-			if (!parent->GetEnableActiveChecks())
-				continue;
-
-			if (parent->GetNextCheck() >= now + parent->GetRetryInterval()) {
-				ObjectLock olock(parent);
-				parent->SetNextCheck(now);
-			}
-		}
-	}
 
 	OnNewCheckResult(this, cr, origin);
 
@@ -506,7 +458,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
 		 * stash them into a notification types bitmask for maybe re-sending later.
 		 */
 
-		ObjectLock olock (this);
 		int suppressed_types_before (GetSuppressedNotifications());
 		int suppressed_types_after (suppressed_types_before | suppressed_types);
 
@@ -536,6 +487,44 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
 	if ((stateChange || hardChange) && !children.empty())
 		OnReachabilityChanged(this, cr, children, origin);
 
+	olock->Unlock();
+	if (!service && problem_change) {
+		auto services = host->GetServices();
+		for (auto& service : services) {
+			Service::OnHostProblemChanged(service, cr, origin);
+		}
+	}
+
+	if (recovery) {
+		for (auto& child : children) {
+			if (child->GetProblem() && child->GetEnableActiveChecks()) {
+				auto nextCheck (now + Utility::Random() % 60);
+
+				ObjectLock oLock (child);
+
+				if (nextCheck < child->GetNextCheck()) {
+					child->SetNextCheck(nextCheck);
+				}
+			}
+		}
+	}
+
+	if (stateChange) {
+		/* reschedule direct parents */
+		for (const Checkable::Ptr& parent : GetParents()) {
+			if (parent.get() == this)
+				continue;
+
+			if (!parent->GetEnableActiveChecks())
+				continue;
+
+			if (parent->GetNextCheck() >= now + parent->GetRetryInterval()) {
+				ObjectLock olock(parent);
+				parent->SetNextCheck(now);
+			}
+		}
+	}
+
 	return Result::Ok;
 }