Skip to content

Keep object locked until events are dispatched. #10372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ vigiroux <[email protected]>
Vytenis Darulis <[email protected]>
Wenger Florian <[email protected]>
Will Frey <[email protected]>
William Calliari <[email protected]>
Winfried Angele <[email protected]>
Wolfgang Nieder <[email protected]>
XnS <[email protected]>
Expand Down
79 changes: 34 additions & 45 deletions lib/icinga/checkable-check.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,8 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
VERIFY(cr);
VERIFY(producer);

{
ObjectLock olock(this);
m_CheckRunning = false;
}
ObjectLock olock(this);
m_CheckRunning = false;

double now = Utility::GetTime();

Expand Down Expand Up @@ -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.
bool affectsPreviousStateChildren(reachable && AffectsChildren());

ObjectLock olock(this);

CheckResult::Ptr old_cr = GetLastCheckResult();
ServiceState old_state = GetStateRaw();
StateType old_stateType = GetStateType();
Expand Down Expand Up @@ -333,8 +329,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());

Expand All @@ -350,8 +344,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr

cr->SetVarsAfter(vars_after);

olock.Lock();

if (service) {
SetLastCheckResult(cr);
} else {
Expand All @@ -361,11 +353,9 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr

if (GetProblem() != wasProblem) {
auto services = host->GetServices();
olock.Unlock();
for (auto& service : services) {
Service::OnHostProblemChanged(service, cr, origin);
}
olock.Lock();
}
}

Expand Down Expand Up @@ -399,8 +389,6 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
}
}

olock.Unlock();

#ifdef I2_DEBUG /* I2_DEBUG */
Log(LogDebug, "Checkable")
<< "Flapping: Checkable " << GetName()
Expand All @@ -411,36 +399,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);

/* signal status updates to for example db_ido */
Expand Down Expand Up @@ -521,7 +479,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);

Expand Down Expand Up @@ -551,6 +508,38 @@ Checkable::ProcessingResult Checkable::ProcessCheckResult(const CheckResult::Ptr
if ((stateChange || hardChange) && !children.empty() && (affectsPreviousStateChildren || AffectsChildren()))
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;
}

Expand Down
Loading