Repository navigation
core: defer SIGUSR2 force update to the main loop - #1062
PeterDessev wants to merge 1 commit into
Conversation
Running the force update in signal context takes a mutex and allocates, which can deadlock the main loop and races the resource manager. Defer to the main loop via a self-pipe and enqueueForceUpdateTimers().
hyprlock runs SIGUSR2 force updates inside the signal handler. face-scan-key sends SIGUSR2 when a scan ends, which is also when PAM succeeds and hyprlock redraws. A signal that lands while the main thread is inside getpwuid() makes the handler wait on that lock forever, so the lock screen hangs after a successful unlock. Carry hyprwm/hyprlock#1062, which defers the update to the main loop through a self-pipe. It does not apply to 0.9.5, so build main.
|
Independent confirmation of the deadlock, plus two related findings from testing this PR. A third lock in the field:
|
| build | ~kHz bursts | ~50 Hz for 3–8 s |
|---|---|---|
| v0.9.5 | 4/4 aborted (malloc(): corrupted top size) within ~300 signals |
1/7 deadlocked (main thread in the handler, label frozen) |
main (9de8719) + this PR |
5/5 survived (5.6k–36k signals) | 7/7 survived, label refreshed on every signal |
With the PR, single signals a second apart (the real-world pattern) refreshed the label 5/5. We now carry this PR on top of main in our NixOS config.
Finding 1: SIGUSR1 deadlocks the same way
As noted under "Not addressed", handleUnlockSignal is still unsafe. It is reachable: after a burst, a single SIGUSR1 left the main thread asleep on a futex with only SIGUSR1 blocked (inside the handler, after logging "Unlocking with a SIGUSR1"), 3/3 times. The same self-pipe could carry an unlock request.
Finding 2: force updates re-fire cancelled timers
enqueueForceUpdateTimers() calls every timer with canForceUpdate(), including ones already cancel()ed in the same processTimers() pass. They are only erased after the pass, and each call replants a label timer through CLabel::onTimer. When several force-update timers fire in one pass, the label timer set multiplies. After a kHz burst, one run logged 1.2M Trying to update label, but a resource is still pending! warnings for ~24k signals, and spaced SIGUSR2s afterwards refreshed the label 0–1 out of 5. This is pre-existing (0.9.5 can't survive the burst to show it) and not reachable at normal signal rates. Skipping t->cancelled() timers in that loop, and taking timersMutex in getTimers(), look like the minimal fixes.
Summary
handleForceUpdateSignalperforms the entire force-update inside the signalhandler. That is not async-signal-safe, and it causes two distinct failures:
label updates being silently dropped (#1031), and — less often but much worse —
the lock screen deadlocking outright.
This moves the work to the main loop via a self-pipe, reusing the existing
enqueueForceUpdateTimers()deferral.The problem
Signal delivery interrupts a thread at an arbitrary instruction and runs the
handler nested inside whatever that thread was doing. This handler takes
m_sLoopState.timersMutex, allocates (thegetTimers()copy), and re-enterswidget code.
Dropped updates (#1031).
CLabel::onTimerUpdate()skips whenm_pendingResourceis set, and that flag is cleared by the resource manager onthe main loop. Firing updates from a signal means observing that state at a
random point — often mid-flight, and possibly inside
onAssetUpdate()betweenclearing the flag and assigning the asset. Which labels happen to have a request
outstanding at that instruction is chance, which matches the report in #1031:
"sometimes only one of them does at a time. Which one updates is not
deterministic."
Deadlock. If the signal lands while the interrupted thread is inside
mallocor already holdstimersMutex, the handler blocks on a lock only thatthread could release. The lock screen freezes with the main thread parked in
futex_do_waitat 0% CPU and no way to authenticate. This is rare per signalbut reachable in seconds by anything that force-updates at a few Hz.
The fix
The handler does the only async-signal-safe thing available —
write()a byteto a self-pipe. The read end joins the existing pollfd array, so the poll thread
wakes the main loop exactly as it already does for wayland and dbus, and the
main loop calls
enqueueForceUpdateTimers().That function already existed and is already used by five call sites in the auth
and fingerprint paths for precisely this reason — deferring widget work to the
main loop. The signal handler was the one place bypassing it.
Why a self-pipe rather than a
sig_atomic_tflagThe poll thread
continues onEINTRwithout notifying the main loop, so abare flag would not be observed until at worst the loop's 5 s
wait_forexpired.The pipe makes the wake deterministic and reuses the existing plumbing.
The drain is deliberately not gated on
revents:reventsis written by thepoll thread and read from the main loop, so testing it there would be racy.
Draining is cheap and idempotent, so the pipe is simply read every iteration.
Drive-by
pollfds[1].reventswas read even when dbus was absent, in which case that slotwas never initialised. The fd indices are now tracked explicitly.
Testing
Built and run on Hyprland 0.56.2 with a widget config using multiple
cmd[update:…:true]+onclicklabels plus a force-updated image, driven byan external daemon sending SIGUSR2 on state changes.
quickly), confirmed as a deadlock — 0 CPU, main thread in
futex_do_wait,hyprlock holding no pipes and no children.
stalls, timer count bounded at 23–29.
Relationship to #1031
This removes the nondeterministic skips. It does not eliminate skipping
entirely: an update that arrives while a command genuinely has not finished yet
is still dropped by the
m_pendingResourceguard, which is the guard doing itsjob. Measured after this patch:
So the residual is purely a function of force-updating faster than the commands
complete, which is a separate concern from the race fixed here. I have not
reproduced #1031's specific setup (multiple labels plus a slow image reload), so
I would rather the reporter confirm than close it from here. Happy to follow up
with a change that coalesces a pending update instead of dropping it, if that
behaviour is wanted.
Not addressed
handleUnlockSignalhas the same class of problem — it callsstd::vformatandthe logger from signal context. Less severe, since unlocking is one-shot and
terminal, but it is the same bug. Left out to keep this to one concern; happy to
fix it here or separately.
🤖 Drafted with Claude Code