tyler.durden/icinga2
1
0
Fork 0
mirror of https://github.com/Icinga/icinga2.git synced 2025-07-26 23:24:09 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #10372 from WuerthPhoenix/10364-race-condition-while-calculating-object-state

Browse Source 
Keep object locked until events are dispatched.
This commit is contained in:
Julian Brost 2025-06-06 17:19:21 +02:00 committed by GitHub
commit a15b706ee3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 35 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
AUTHORS
View File

@ -301,6 +301,7 @@ vigiroux <vincent.giroux@nokia.com>
Vytenis Darulis <vytenis@uber.com> Vytenis Darulis <vytenis@uber.com>
Wenger Florian <wenger@unifox.at> Wenger Florian <wenger@unifox.at>
Will Frey <will.frey@digitalreasoning.com> Will Frey <will.frey@digitalreasoning.com>
William Calliari <42240136+w1ll-i-code@users.noreply.github.com>
Winfried Angele <winfried.angele@gmail.com> Winfried Angele <winfried.angele@gmail.com>
Wolfgang Nieder <wnd@gmx.net> Wolfgang Nieder <wnd@gmx.net>
XnS <git@xns.be> XnS <git@xns.be>

75
lib/icinga/checkable-check.cpp
View File

@ -104,10 +104,8 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
VERIFY(cr); VERIFY(cr);
VERIFY(producer); VERIFY(producer);
{
ObjectLock olock(this); ObjectLock olock(this);
m_CheckRunning = false; m_CheckRunning = false;
}
double now = Utility::GetTime(); double now = Utility::GetTime();
@ -169,8 +167,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
// This will be used to determine whether the on reachability changed event should be triggered. // This will be used to determine whether the on reachability changed event should be triggered.
bool affectsPreviousStateChildren(reachable && AffectsChildren()); bool affectsPreviousStateChildren(reachable && AffectsChildren());
ObjectLock olock(this);
CheckResult::Ptr old_cr = GetLastCheckResult(); CheckResult::Ptr old_cr = GetLastCheckResult();
ServiceState old_state = GetStateRaw(); ServiceState old_state = GetStateRaw();
StateType old_stateType = GetStateType(); StateType old_stateType = GetStateType();
@ -333,8 +329,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
if (is_volatile && IsStateOK(old_state) && IsStateOK(new_state)) if (is_volatile && IsStateOK(old_state) && IsStateOK(new_state))
send_notification = false; /* Don't send notifications for volatile OK -> OK changes. */ send_notification = false; /* Don't send notifications for volatile OK -> OK changes. */
olock.Unlock();
if (remove_acknowledgement_comments) if (remove_acknowledgement_comments)
RemoveAckComments(String(), cr->GetExecutionEnd()); RemoveAckComments(String(), cr->GetExecutionEnd());
@ -350,8 +344,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
cr->SetVarsAfter(vars_after); cr->SetVarsAfter(vars_after);
olock.Lock();
if (service) { if (service) {
SetLastCheckResult(cr); SetLastCheckResult(cr);
} else { } else {
@ -361,11 +353,9 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
if (GetProblem() != wasProblem) { if (GetProblem() != wasProblem) {
auto services = host->GetServices(); auto services = host->GetServices();
olock.Unlock();
for (auto& service : services) { for (auto& service : services) {
Service::OnHostProblemChanged(service, cr, origin); Service::OnHostProblemChanged(service, cr, origin);
} }
olock.Lock();
} }
} }
@ -399,8 +389,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
} }
} }
olock.Unlock();
#ifdef I2_DEBUG /* I2_DEBUG */ #ifdef I2_DEBUG /* I2_DEBUG */
Log(LogDebug, "Checkable") Log(LogDebug, "Checkable")
<< "Flapping: Checkable " << GetName() << "Flapping: Checkable " << GetName()
@ -411,36 +399,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
<< "% current: " << GetFlappingCurrent() << "%."; << "% current: " << GetFlappingCurrent() << "%.";
#endif /* I2_DEBUG */ #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); OnNewCheckResult(this, cr, origin);
/* signal status updates to for example db_ido */ /* signal status updates to for example db_ido */
@ -521,7 +479,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
* stash them into a notification types bitmask for maybe re-sending later. * stash them into a notification types bitmask for maybe re-sending later.
*/ */
ObjectLock olock (this);
int suppressed_types_before (GetSuppressedNotifications()); int suppressed_types_before (GetSuppressedNotifications());
int suppressed_types_after (suppressed_types_before | suppressed_types); int suppressed_types_after (suppressed_types_before | suppressed_types);
@ -551,6 +508,38 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
if ((stateChange || hardChange) && !children.empty() && (affectsPreviousStateChildren || AffectsChildren())) if ((stateChange || hardChange) && !children.empty() && (affectsPreviousStateChildren || AffectsChildren()))
OnReachabilityChanged(this, cr, children, origin); OnReachabilityChanged(this, cr, children, origin);
olock.Unlock();
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; return Result::Ok;
} }