LifetimeManager: guard s_OnTimeout against unmatched timer fires (#40449)
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: Ben Hillis <benhill@ntdev.microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ben Hillis committed
May 7, 2026 at 17:58 UTC
c985463c7205e7b6c2d3404f33369606485c1d39
1 file changed
+10
-6
src/windows/service/exe/Lifetime.cpp
+10
-6
@@ -236,12 +236,16 @@ VOID CALLBACK LifetimeManager::s_OnTimeout(_Inout_ PTP_CALLBACK_INSTANCE, _Inout
236
manager->m_callbackList.erase(client);
237
}
238
239
- // The destructor has not run because m_callbackList is not empty.
240
- // Put the current timer in previousTimerWait to make sure the destructor waits for us if
241
- // it runs after we drop manager->m_lock.
242
- clientLocal.CancelTimer();
243
- previousTimerWait.reset(clientLocal.timer.release());
244
- previousTimerWait.swap(manager->m_lastTimerWait);
239
+ // If we took ownership of the timer (moved into clientLocal), stash it so the
240
+ // destructor waits for this callback to finish before exiting. Otherwise this
241
+ // firing is a no-op (e.g., the entry was erased, the timer was replaced, or the
242
+ // matched entry still has live processes).
243
+ if (clientLocal.timer)
244
+ {
245
+ clientLocal.CancelTimer();
246
+ previousTimerWait.reset(clientLocal.timer.release());
247
+ previousTimerWait.swap(manager->m_lastTimerWait);
248
+ }
249
}
250
251
// If a callback was found, execute it. If the callback succeeds the