fix: guard against layer-surface buffer-before-ack protocol error

[AvengerAnubis]

Problem

Noctalia (via quickshell) can commit a buffer to a zwlr_layer_surface_v1 before acknowledging the initial configure event. Smithay's layer-shell commit handler treats this as InvalidSurfaceState protocol error and kills the client (noctalia). This manifests as random crashes — especially when panels (clipboard history, etc.) are opened and closed rapidly.

Root cause

Race in quickshell's layer-surface lifecycle — it attaches a buffer in the same dispatch cycle as the initial commit, before the configure event from the compositor has been processed and acknowledged.

Fix

A pre-commit hook (registered before smithay's validation hook in new_surface) that reads last_acked from the layer-surface role (LayerSurfaceData). If last_acked is None (configure not yet acknowledged) and the pending state contains a NewBuffer, the buffer is removed (set to Removed). Smithay's commit handler then sees no buffer, skips the protocol error, and the client safely acks the configure and retries the buffer on the next commit.

Notes

• Only fires for surfaces with LayerSurfaceData — regular xdg-toplevel windows are unaffected.
• Reads last_acked from the role data (LayerSurfaceAttributes via LayerSurfaceData), not from LayerSurfaceCachedState.pending() — the role's last_acked is updated immediately by ack_configure, while the cached state is only synced on the next commit.
• Other compositors (wlroots, Hyprland) handle this case leniently — they ignore the premature buffer rather than killing the client. This patch adopts the same behavior.

[AvengerAnubis]

I'm gonna fix the CI errors, will push the fix commit soon

[AvengerAnubis]

Tested on my machine with Noctalia, seems to be working just fine. Before it (Noctalia in my case) was crashing every 5-10 minutes, now it's going with no crashes for 20 minutes at least

[AvengerAnubis]

the commit e5115b7 presumably fixes crush of quickshell/Noctalia etc. when exiting lockscreen (entering the right password). Sadly I can't really tell if the bug was there before or if it's been introduced by PR, but with the e5115b7 commit fix, no crushes of Noctalia so far

[malbiruk]

Thanks for digging into this -- the diagnosis was on the right track, and it led me to a real bug one layer deeper than the patch.

The root cause is on our side: layer_destroyed never removed the surface from the output's LayerMap (zero unmap_layer calls in the whole tree -- stale entries were only reaped once per rendered frame in post_render). So a client that destroys its zwlr_layer_surface_v1 and takes a new one on the same wl_surface before the next frame leaves two entries in the map; lookups match by wl_surface in map order, find the dead one, and the initial configure goes out on a destroyed proxy while marking the shared attributes as configured. The client never hears a size, then gets killed when it attaches a buffer, which is exactly why it clustered on rapid panel open/close. niri and cosmic-comp both unmap synchronously in layer_destroyed, which is why noctalia is stable there; we were the outlier.

Fix and a regression test that reproduces the destroy-recreate sequence are on layer-destroy-unmap. Could you re-test with that and without your patch? If the crashes stop I'll close this as fixed differently -- you found the bug, and I'd like to credit you on it.

[AvengerAnubis]

Tested with your layer-destroy-unmap plus my local changes regarding dbus support for my particular use case (those shouldn't be affecting the test). Still crushes at random moments.
Noctalia crush log:

23:14:07.175 [ERR] fatal: failed to dispatch pending Wayland events before poll: display_error=32 (Broken pipe)

driftwm logs that are probably related to the crush (output from dev version from my fork, the logger was added specificaly to catch this crush, which has been removed in last commit here):

client=InnerClientId { id: 2, serial: 3 } disconnected due to protocol error: interface=ext_session_lock_surface_v1 code=0 msg=Committed before the first ack_configure.
client=InnerClientId { id: 2, serial: 3 } disconnected due to protocol error: interface=zwlr_layer_surface_v1 code=0 msg=must ack the initial configure before attaching buffer

I think the PR #1991 (and the conversation there) may be related to this issue.
I'm gonna dig deeper into this tonight, and will commit a proper fix instead of the current workaround here, if I find one.

[malbiruk]

Thanks, so:

smithay never unregisters its layer-shell pre-commit hook, and destroyed zeroes the role state, so later commits on that wl_surface validate against wiped state and post on a dead proxy. We already forced full anchors there, which covers the InvalidSize branches only -- the buffer-before-ack branch your patch neutralises was wide open. That's why your build is stable and my branch alone wasn't. I've gated it on "the role is dead" rather than last_acked.is_none(), so a premature buffer on a live role still errors, plus regression tests.

Updated branch: layer-destroy-unmap. Could you re-test with it and without your patch?

[AvengerAnubis]

I tested it and sadly, noctalia still crashes with the same log message:
01:56:03.467 [ERR] fatal: failed to dispatch pending Wayland events before poll: display_error=32 (Broken pipe)
Do I update this branch? or do you have an idea on how to fix this?
Let me know what you think

[malbiruk]

I'm so sorry man, I forgot to push 😭

I also made driftwm log protocol-error disconnects itself; if it still dies, paste the line that reads "client ... disconnected: @ code=N — ".

If it says ext_session_lock_surface_v1, that's the unlock crash probably upstream #1991 (Smithay/smithay#1991), and separate from this PR. If it says zwlr_layer_surface_v1, the fix missed a path, most likely the role is still orphaned but the marker never got set, rather than a live-role violation (noctalia survives on niri, which configures on the first commit just like we do). I'd chase that first, and fall back to your broader last_acked.is_none() condition if it turns up nothing.

[AvengerAnubis]

Thats fine :D it happens to me too.
So I've been running it for around 20+ minutes straight and no crashes so far (before it was happening either almost instantly or in 10 minutes I think).
I think it's fixed now, thanks!

[malbiruk]

I'm glad, pushed to main, thanks for diagnosing and sticking with me for testing! Closing this PR as fixed differently