Skip to content

Commit 1204d40

Browse files
w1ll-i-codeWilliam Calliari
authored and
William Calliari
committed
Keep object locked until events are dispatched.
1 parent 8d607d2 commit 1204d40

File tree

1 file changed

+42
-54
lines changed

1 file changed

+42
-54
lines changed

lib/icinga/checkable-check.cpp

Lines changed: 42 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,8 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
9898
{
9999
using Result = Checkable::ProcessingResult;
100100

101-
{
102-
ObjectLock olock(this);
103-
m_CheckRunning = false;
104-
}
101+
ObjectLock olock(this);
102+
m_CheckRunning = false;
105103

106104
if (!cr)
107105
return Result::NoCheckResult;
@@ -158,8 +156,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
158156
// This will be used to determine whether the on reachability changed event should be triggered.
159157
bool affectsPreviousStateChildren(reachable && AffectsChildren());
160158

161-
ObjectLock olock(this);
162-
163159
CheckResult::Ptr old_cr = GetLastCheckResult();
164160
ServiceState old_state = GetStateRaw();
165161
StateType old_stateType = GetStateType();
@@ -322,8 +318,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
322318
if (is_volatile && IsStateOK(old_state) && IsStateOK(new_state))
323319
send_notification = false; /* Don't send notifications for volatile OK -> OK changes. */
324320

325-
olock.Unlock();
326-
327321
if (remove_acknowledgement_comments)
328322
RemoveAckComments(String(), cr->GetExecutionEnd());
329323

@@ -339,25 +333,14 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
339333

340334
cr->SetVarsAfter(vars_after);
341335

342-
olock.Lock();
343-
336+
bool problem_change = false;
344337
if (service) {
345338
SetLastCheckResult(cr);
346339
} else {
347340
bool wasProblem = GetProblem();
348-
349341
SetLastCheckResult(cr);
350-
351-
if (GetProblem() != wasProblem) {
352-
auto services = host->GetServices();
353-
olock.Unlock();
354-
for (auto& service : services) {
355-
Service::OnHostProblemChanged(service, cr, origin);
356-
}
357-
olock.Lock();
358-
}
342+
problem_change = GetProblem() != wasProblem;
359343
}
360-
361344
bool was_flapping = IsFlapping();
362345

363346
UpdateFlappingStatus(cr->GetState());
@@ -388,8 +371,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
388371
}
389372
}
390373

391-
olock.Unlock();
392-
393374
#ifdef I2_DEBUG /* I2_DEBUG */
394375
Log(LogDebug, "Checkable")
395376
<< "Flapping: Checkable " << GetName()
@@ -400,36 +381,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
400381
<< "% current: " << GetFlappingCurrent() << "%.";
401382
#endif /* I2_DEBUG */
402383

403-
if (recovery) {
404-
for (auto& child : children) {
405-
if (child->GetProblem() && child->GetEnableActiveChecks()) {
406-
auto nextCheck (now + Utility::Random() % 60);
407-
408-
ObjectLock oLock (child);
409-
410-
if (nextCheck < child->GetNextCheck()) {
411-
child->SetNextCheck(nextCheck);
412-
}
413-
}
414-
}
415-
}
416-
417-
if (stateChange) {
418-
/* reschedule direct parents */
419-
for (const Checkable::Ptr& parent : GetParents()) {
420-
if (parent.get() == this)
421-
continue;
422-
423-
if (!parent->GetEnableActiveChecks())
424-
continue;
425-
426-
if (parent->GetNextCheck() >= now + parent->GetRetryInterval()) {
427-
ObjectLock olock(parent);
428-
parent->SetNextCheck(now);
429-
}
430-
}
431-
}
432-
433384
OnNewCheckResult(this, cr, origin);
434385

435386
/* signal status updates to for example db_ido */
@@ -510,7 +461,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
510461
* stash them into a notification types bitmask for maybe re-sending later.
511462
*/
512463

513-
ObjectLock olock (this);
514464
int suppressed_types_before (GetSuppressedNotifications());
515465
int suppressed_types_after (suppressed_types_before | suppressed_types);
516466

@@ -540,6 +490,44 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
540490
if ((stateChange || hardChange) && !children.empty() && (affectsPreviousStateChildren || AffectsChildren()))
541491
OnReachabilityChanged(this, cr, children, origin);
542492

493+
olock->Unlock();
494+
if (!service && problem_change) {
495+
auto services = host->GetServices();
496+
for (auto& service : services) {
497+
Service::OnHostProblemChanged(service, cr, origin);
498+
}
499+
}
500+
501+
if (recovery) {
502+
for (auto& child : children) {
503+
if (child->GetProblem() && child->GetEnableActiveChecks()) {
504+
auto nextCheck (now + Utility::Random() % 60);
505+
506+
ObjectLock oLock (child);
507+
508+
if (nextCheck < child->GetNextCheck()) {
509+
child->SetNextCheck(nextCheck);
510+
}
511+
}
512+
}
513+
}
514+
515+
if (stateChange) {
516+
/* reschedule direct parents */
517+
for (const Checkable::Ptr& parent : GetParents()) {
518+
if (parent.get() == this)
519+
continue;
520+
521+
if (!parent->GetEnableActiveChecks())
522+
continue;
523+
524+
if (parent->GetNextCheck() >= now + parent->GetRetryInterval()) {
525+
ObjectLock olock(parent);
526+
parent->SetNextCheck(now);
527+
}
528+
}
529+
}
530+
543531
return Result::Ok;
544532
}
545533

0 commit comments

Comments
 (0)