Skip to content

LifetimeManager: guard s_OnTimeout against unmatched timer fires#40449

Merged
benhillis merged 1 commit intomasterfrom
user/benhill/lifetime_ontimeout_fix
May 8, 2026
Merged

LifetimeManager: guard s_OnTimeout against unmatched timer fires#40449
benhillis merged 1 commit intomasterfrom
user/benhill/lifetime_ontimeout_fix

Conversation

@benhillis
Copy link
Copy Markdown
Member

@benhillis benhillis commented May 7, 2026

If the timer fires but find_if doesn't match (entry erased, or replaced by the retry path), clientLocal is default-constructed and the unconditional swap clobbers m_lastTimerWait with null - breaking the destructor's wait-for-pending-callbacks guarantee.

Guard the swap so an unmatched fire is a no-op.

Copilot AI review requested due to automatic review settings May 7, 2026 00:28
@benhillis benhillis requested a review from a team as a code owner May 7, 2026 00:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a race in LifetimeManager::s_OnTimeout where an unmatched timer fire could cause m_lastTimerWait to be overwritten with nullptr, breaking the destructor’s “wait for pending callbacks” guarantee in the WSL Windows service lifetime tracking.

Changes:

  • Guarded the timer release()/reset()/swap() sequence so it only runs when the timed-out callback entry was actually found (i.e., clientLocal.timer is non-null).
  • Added an explanatory comment documenting why the guard is needed.

Comment thread src/windows/service/exe/Lifetime.cpp Outdated
@benhillis benhillis force-pushed the user/benhill/lifetime_ontimeout_fix branch 2 times, most recently from 8880f27 to 8fe54d6 Compare May 7, 2026 00:36
Copilot AI review requested due to automatic review settings May 7, 2026 00:36
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread src/windows/service/exe/Lifetime.cpp Outdated
When the timer callback runs but find_if doesn't match (because the entry
was already removed and recreated by RefreshClientCallback or similar),
clientLocal stays default-constructed with a null timer. The unconditional
.release()/.swap() then clobbers any previously stashed m_lastTimerWait
with null, breaking the destructor's wait guarantee. Skip the swap when
there is no timer to stash.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@benhillis benhillis force-pushed the user/benhill/lifetime_ontimeout_fix branch from 8fe54d6 to ef105d3 Compare May 7, 2026 00:42
@benhillis benhillis merged commit c985463 into master May 8, 2026
11 checks passed
@benhillis benhillis deleted the user/benhill/lifetime_ontimeout_fix branch May 8, 2026 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants