Consistently catch LSP panics

[lionel-]

This PR revisits how LSP panics are caught to:

• Catch panics produced in notification handlers and other parts of the LSP service. Previously only request handlers were properly guarded. Addresses A panic outside the LSP request path disables the R language server with no message positron#15666

• Catch panics on the I/O and analysis task pools without aborting the process. Addresses A panicking analysis task aborts the whole R session instead of being caught positron#15667

Special care is taken for:

• Salsa cancellation panics, which should not be reported as crash but simply advance the LSP loop or close the background task.
• Panics and cancellations across an r_task(). This is mainly defensive, but r_task() is used in legacy parts of the LSP so we better be safe.

Unlike panics in request handlers, panics in notification handlers are unrecoverable. See these screencasts, the first causes a notification panic and shuts down the LSP in an orderly fashion after notifying the user:

Screen.Recording.2026-09-11.at.16.24.33.mov

The second is a request panic that only notifies the user and keeps the LSP in working order:

Screen.Recording.2026-09-11.at.16.25.36.mov

To avoid spamming the user with notifications, panics are only reported once per handler. They keep being logged in the background so user reports are complete.

When a panic occurs in a background task (e.g. diagnostics), a notification is sent once. Further panics are logged only.

Screen.Recording.2026-09-11.at.16.51.18.mov

Positron Release Notes

New Features

• N/A

Bug Fixes

• N/A

[juliasilge]

I want to make sure I understand what's going on here. The old code caught unconditionally, right? A panic with no recovering caller was caught on the R thread, ferried back, and re-raised on the calling thread? The hook then saw no boundary and aborted. The new code aborts at the original panic site instead. Both paths abort, so is there a case where the difference is visible?

I ask because the condition removes the guard that the old comment called out: "a Rust unwind crossing those C frames is UB". Can we keep catch_unwind unconditional and decide whether to abort at the re-raise site? That would keep the guarantee from the old comment, and AFACT it gives the same observable behavior.

[lionel-]

@juliasilge Assuming this is about r_task(), before I implemented this catch site in 4de75ff, we were not catching anything. In that case the panic hook runs its full course with specific dispositions for the R thread (backtrace capture) and aborts the process. There is no unwinding or crossing of C frames. The catching and rethrow was specifically implemented for Salsa cancellations.

I'd say the new behaviour of checking if there is a recovery boundary is marginally better because of the immediate process exit in case of panic. I also like that this unifies r_task() and Console::with() recovery mechanisms.

Are you concerned about other kinds of resume_unwind() throwing (same mechanism as Salsa cancellations) which does not invoke the panic hook? I'd says those are UB (from our own policy standpoint, not the language) to throw on the R thread. The Salsa situation is a temporary stopgap. If we wanted to do better here, we'd have to consider other boundaries like try_catch() and that seems daunting. For instance I've stayed clear of Rust stack unwinding in #1309 for this reason.

[lionel-]

Worth noting that in parallel to that commit for catching salsa cancellations we've started catching LSP panics. And since the legacy LSP is the main consumer of r_task (now that comms run directly on the R threads) it should be unexpected to go through that immediately aborting path, since we are now consistently installing recovery boundaries.

Also since resume_unwind doesn't reinvoke the panic hook, re throwing panics unconditionally would result in a greater risk of a thread silently dying. Although we've been heading in the opposite direction of keeping the process alive in case of panics, we should arguably only recover in controlled situations rather than let the process in a partially working state.

[juliasilge]

Ah, thank you! I was not thinking about this quite correctly. I overlooked that the panic hook runs before unwinding, so an ordinary panic in the old code would abort on the R thread before the unconditional catch_unwind() could ferry it back.

The conditional catch makes sense given that distinction, and I see why rethrowing without a known recovery boundary risks silently losing the calling thread. Do you think it may be worth documenting the remaining invariant? IIUC, it's that an r_task() without a recovering caller must not originate a hookless unwind such as resume_unwind(), since that could cross the r_sandbox() C frames.

But this looks good to me!

[thomasp85]

Not going to contest the review by Julia. One observation though which may be due to my lack of understanding of the life cycle.

LSP_HAS_CRASHED/BACKGROUND_PANIC_REPORTED is set once and never reset, so an LSP restart (e.g. reloading the window) will inherit the crashed state. You have to restart ark to recover. If this is intended or not a concern then all good

[lionel-]

@thomasp85 I thought of both of these flags as anti-spam measures:

• The global crash flag would prevent the client from reconnecting repeatedly
• The background panic report flag would prevent a repeatedly failing handler from spamming the user

However after taking another look, I think both of these were a bit off.

For the global flag, it turns out the client doesn't repeatedly reconnect. I think because it only sees a disconnection, not a process crash. Instead, what you see when switching R sessions (which causes the LSPs to cycle), is repeated crash notifications:

Screen.Recording.2026-09-16.at.10.46.09.mov

So it's the worst of both worlds. We don't guard against repeated reconnections, and we prevent the LSP from restarting even though the cause of the crash might not be a repeating event.

I've changed that flag to only disable LSP features while the LSP is tearing down. So if the LSP is repeatedly crashing down, you'll still see repeated notifications as the LSPs cycle, but if the crash was transient, they will work as expected. Here is what you now see when the crash is not transient:

Screen.Recording.2026-09-16.at.10.41.19.mov

Regarding the background panic flag, it seems fine to reset it when the LSP reconnects. That would already prevent most of the spamming. Now done:

Screen.Recording.2026-09-16.at.10.46.09.mov

And a nice outcome is that now both of these flags are now local state of an LSP session, not global flags.

Thanks for pointing this out!