Skip to content

Make the Windows handler reentrancy-safe (fix "already borrowed" panic on resize)#263

Closed
aidan729 wants to merge 1 commit into
RustAudio:masterfrom
aidan729:fix/win-handler-reentrancy
Closed

Make the Windows handler reentrancy-safe (fix "already borrowed" panic on resize)#263
aidan729 wants to merge 1 commit into
RustAudio:masterfrom
aidan729:fix/win-handler-reentrancy

Conversation

@aidan729

Copy link
Copy Markdown

Problem

On Windows, WindowState::handle_on_frame and handle_event hold
self.handler.borrow_mut() for the full duration of the user's on_frame /
on_event callback. If the callback causes Windows to synchronously dispatch
another message into the window procedure, the wndproc re-enters handle_event,
calls borrow_mut() again, and panics with already borrowed: BorrowMutError.

The most common trigger is resizing: calling a host's resize request (or a
SetWindowPos) from inside a handler callback synchronously sends WM_SIZE back
into the window proc. In an audio-plugin host this panic unwinds across the FFI
boundary and crashes the DAW.

This is reproducible whenever a baseview-hosted GUI tries to resize its own
window in response to user interaction (e.g. a draggable resize affordance that
asks the host to grow the editor).

Fix

Use try_borrow_mut() in both handle_on_frame and handle_event and skip the
event if the handler is already borrowed, instead of panicking. The handler is
not reentrant, so dropping the nested call is the correct behavior — the window
state remains consistent and the next genuine frame/event observes the latest
size.

This brings the Windows backend in line with how non-reentrant event loops are
expected to behave and makes it safe to invoke host resize APIs from within a
handler callback.

Scope

  • Windows only (src/win/window.rs), two call sites.
  • No public API change; no behavior change except that a previously-fatal
    re-entrant callback is now skipped instead of panicking.

handle_on_frame and handle_event hold self.handler.borrow_mut()
across the entire user callback. If that callback synchronously triggers
another window message — e.g. a SetWindowPos or host-driven resize that
sends WM_SIZE straight back into the window procedure — the wndproc
re-enters handle_event, borrows the handler a second time, and panics
with a BorrowMutError. In an audio-plugin context that unwinds across the
FFI boundary and crashes the host.

Use 	ry_borrow_mut() and skip the re-entrant call instead of panicking.
The handler isn't reentrant, so dropping the nested invocation is correct:
window state stays consistent and the next genuine frame/event picks up
the latest size. This makes it safe to call host resize APIs (which
synchronously re-enter the window proc on Windows) from inside a handler
callback.
@aidan729

Copy link
Copy Markdown
Author

For additional context, someone in the Rust Audio Discord raised a concern about this approach:

"I don't think it's a good idea to throw away events when you get re-entrancy."

My response was that I don't believe anything meaningful is actually being dropped in this case. The re-entrant call isn't carrying new independent input; it's a synchronous side effect of the outer callback. Specifically, the resize API call immediately generates a WM_SIZE and re-enters the window procedure before the original call returns.

As a result, the nested invocation observes handler state while it is mid-mutation, which is exactly why the borrow_mut() panic occurs. For the resize path, window size is effectively level-triggered rather than edge-triggered, the current size remains available as window state and is not a one-shot event that must be consumed immediately. The next normal on_frame / on_event invocation will observe the latest size and reconcile accordingly.

From that perspective, skipping the nested invocation is simply deferring reconciliation until the borrow is valid rather than panicking and potentially unwinding across the FFI boundary into the host.

The main alternative would be to queue re-entrant events and replay them once the borrow is released. I considered that approach, but for WM_SIZE specifically, the event is often already superseded by the time it would be replayed, which introduces ordering and deduplication complexity without an obvious benefit.

That said, if there's a case where a skipped re-entrant invocation carries state that cannot otherwise be recovered, I'd be interested in seeing an example. I wasn't able to construct such a case for the resize path.

@prokopyl

Copy link
Copy Markdown
Member

After reviewing all of this, I have to agree that simply dropping events in the case of a re-entrant call is not a good idea.

Even in the relatively "simple" case of window resizing, this introduces additional complexity for the downstream implementations: they have to realize that this window resize won't trigger their own event handler, and trigger it themselves afterwards.

And that's only good for cases where the implementation knows that a specific event will trigger a resize, which is not always the case.

This, and also I believe that the fact that baseview requires &mut references to the handler is just a bad idea in general. It introduces a slew of edge cases and "already borrowed" crashes, all of which we cannot work around in every case. I think it would be better if we got rid of the RefCell altogether and just embraced reentrancy. (I'm experimenting with this idea here: #267 ).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants