core: fix use-after-free segfault in async resource widget callbacks (#961)

Properly lock AWP<IWidget> weak pointers before calling onAssetUpdate()
to prevent use-after-free when widgets are destroyed during shutdown or
output removal. Guard timer callback against null g_asyncResourceManager.
Fix destruction order to join threads before resetting globals.
This commit is contained in:
Sjoerd Siebinga 2026-02-20 14:46:42 +01:00 committed by GitHub
parent ef3017f5ef
commit b3a1076c03
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 29 additions and 15 deletions

View file

@ -452,8 +452,20 @@ void CHyprlock::run() {
const auto DPY = m_sWaylandState.display;
// Wake threads so they observe m_bTerminate and exit; pending timers are dropped
m_sLoopState.timerEvent = true;
m_sLoopState.timerCV.notify_all();
pthread_kill(pollThr.native_handle(), SIGRTMIN);
g_pAuth->terminate();
// Wait for threads to exit before destroying globals to prevent
// use-after-free in timer callbacks referencing g_asyncResourceManager
pollThr.join();
timersThr.join();
// Now safe to destroy globals — no more timer callbacks can fire
m_sWaylandState = {};
dma = {};
@ -465,14 +477,6 @@ void CHyprlock::run() {
wl_display_disconnect(DPY);
pthread_kill(pollThr.native_handle(), SIGRTMIN);
g_pAuth->terminate();
// wait for threads to exit cleanly to avoid a coredump
pollThr.join();
timersThr.join();
Debug::log(LOG, "Reached the end, exiting");
}

View file

@ -248,11 +248,12 @@ bool CAsyncResourceManager::request(ResourceID id, const AWP<IWidget>& widget) {
if (m_assets[id].texture) {
// Asset already present. Dispatch the asset callback function.
const auto& TEXTURE = m_assets[id].texture;
if (widget)
widget->onAssetUpdate(id, TEXTURE);
if (auto w = widget.lock()) {
w->onAssetUpdate(id, TEXTURE);
// TODO: add a centalized mechanism to render in one place in the event loop to avoid duplicate render calls
g_pHyprlock->addTimer(std::chrono::milliseconds(0), [](auto, auto) { g_pHyprlock->renderAllOutputs(); }, nullptr);
// TODO: add a centalized mechanism to render in one place in the event loop to avoid duplicate render calls
g_pHyprlock->addTimer(std::chrono::milliseconds(0), [](auto, auto) { g_pHyprlock->renderAllOutputs(); }, nullptr);
}
} else if (widget) {
// Asset currently in-flight. Add the widget reference to in order for the callback to get dispatched later.
m_resourcesMutex.lock();
@ -279,7 +280,16 @@ void CAsyncResourceManager::enqueue(ResourceID resourceID, const ASP<IAsyncResou
m_resourcesMutex.unlock();
resource->m_events.finished.listenStatic([resourceID]() {
g_pHyprlock->addTimer(std::chrono::milliseconds(0), [](auto, void* resourceID) { g_asyncResourceManager->onResourceFinished((size_t)resourceID); }, (void*)resourceID);
if (!g_pHyprlock)
return;
g_pHyprlock->addTimer(
std::chrono::milliseconds(0),
[](auto, void* resourceID) {
if (g_asyncResourceManager)
g_asyncResourceManager->onResourceFinished((size_t)resourceID);
},
(void*)resourceID);
});
}
@ -331,8 +341,8 @@ void CAsyncResourceManager::onResourceFinished(ResourceID id) {
m_assets[id].texture = texture;
for (const auto& widget : WIDGETS) {
if (widget)
widget->onAssetUpdate(id, texture);
if (auto w = widget.lock())
w->onAssetUpdate(id, texture);
}
g_pHyprlock->renderAllOutputs();