Skip to content

Fix missing crash dialog for panics thrown inside requestAnimationFrame loop#3788

Open
JustJ01 wants to merge 4 commits intoGraphiteEditor:masterfrom
JustJ01:fix-panic-requestanimationframe
Open

Fix missing crash dialog for panics thrown inside requestAnimationFrame loop#3788
JustJ01 wants to merge 4 commits intoGraphiteEditor:masterfrom
JustJ01:fix-panic-requestanimationframe

Conversation

@JustJ01
Copy link
Contributor

@JustJ01 JustJ01 commented Feb 18, 2026

Closes #3402

Panics triggered from code running in the recursive requestedAnimationFrame path could freeze the editor without showing the crash dialog.

This PR makes those panics follow the same crash-reporting flow as other panics.

The Root Cause:

The panic handler could clash while trying to lock EDITOR_HANDLE during requestedAnimationFrame dispatch, so the crash dialog message was not always sent in that path.

Old panic code used a blocking lock (lock()) , so panic handling could get stuck waiting for that same lock, as a result app froze.

What I changed:

  • panic_hook() now uses non blocking strategy:
    • Try send_panic_dialog_via_callback() first.
    • If that fails , try send_panic_dialog(), uses try_lock(), not lock(),
    • If that also fails, schedule send_panic_dialog_deferred() with setTimeout(0) and retry next tick.
  • The raw frontend JS callback is stored separately in thread-local state: PANIC_DIALOG_MESSAGE_CALLBACK, that callback is set during EditorHandle::create(), this path avoids EDITOR_HANDLE mutex contention entirely, so it stills works during requestedAnimationFrame related panic timing.

Verification:

Add panic!() in brightness_contrast_properties() and select Brightness/Contrast node , Crash dialog now appears (no freeze)

Screen.Recording.2026-02-19.011012.mp4

@gemini-code-assist
Copy link

Summary of Changes

Hello @JustJ01, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical stability issue where the editor would become unresponsive instead of showing a crash dialog when a panic occurred within the requestAnimationFrame execution path. The core problem stemmed from mutex contention, specifically with the EDITOR_HANDLE, which prevented the panic handler from acquiring the necessary lock to display the dialog. The solution involves a robust, multi-stage panic handling approach that prioritizes non-blocking communication with the frontend, ensuring that crash reports are reliably presented to the user, thereby improving the application's resilience and user experience during unexpected errors.

Highlights

  • Crash Dialog Fix: Resolved an issue where panics occurring within the requestAnimationFrame loop would cause the editor to freeze without displaying a crash dialog.
  • Non-Blocking Panic Handling: Implemented a new, non-blocking strategy for the panic_hook to ensure crash dialogs are always displayed, even during mutex contention.
  • Mutex Contention Avoidance: Introduced a thread-local PANIC_DIALOG_MESSAGE_CALLBACK to send panic messages directly via a raw JavaScript callback, bypassing potential deadlocks with the EDITOR_HANDLE mutex.
  • Deferred Panic Dialog: Added a fallback mechanism to schedule the crash dialog display using setTimeout(0) if immediate attempts to send the message fail.
Changelog
  • editor/src/messages/portfolio/document/node_graph/node_properties.rs
    • Added a temporary panic!() call for verification purposes during development.
  • frontend/wasm/src/editor_api.rs
    • Imported the new PANIC_DIALOG_MESSAGE_CALLBACK.
    • Modified the EditorHandle::create function to store the frontend_message_handler_callback in the PANIC_DIALOG_MESSAGE_CALLBACK thread-local variable.
  • frontend/wasm/src/lib.rs
    • Imported JsCast for type casting JavaScript values.
    • Declared PANIC_DIALOG_MESSAGE_CALLBACK as a new thread-local RefCell to hold the JavaScript callback for panic messages.
    • Refactored the panic_hook function to implement a new non-blocking, multi-tiered strategy for sending crash dialog messages.
    • Introduced send_panic_dialog_via_callback to attempt sending panic messages using the direct JavaScript callback, avoiding mutex contention.
    • Added send_panic_dialog which uses try_lock() on EDITOR_HANDLE as a secondary attempt.
    • Implemented send_panic_dialog_deferred to schedule a deferred attempt to send the panic dialog using setTimeout(0) if previous attempts fail, with a native-specific no-op for non-web environments.
Activity
  • No human activity to report on this pull request yet.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request effectively resolves the issue of the editor freezing on panics within the requestAnimationFrame loop by introducing a non-blocking, multi-stage panic handling strategy. The approach of using a direct JS callback to bypass mutex contention on EDITOR_HANDLE is a solid solution. The code is well-structured and the changes align with the detailed explanation in the PR description. My review includes one critical comment to remove leftover test code and a couple of medium-severity suggestions for improving code quality and performance.

I am having trouble creating individual review comments. Click here to see my feedback.

editor/src/messages/portfolio/document/node_graph/node_properties.rs (1287)

critical

This panic!() call appears to be for testing purposes to verify the crash dialog behavior. It should be removed before this pull request is merged to prevent intentional crashes in the application.

frontend/wasm/src/editor_api.rs (100-109)

medium

There's significant code duplication between this function and the #[cfg(feature = "native")] version of create below. To improve maintainability and reduce redundancy, consider extracting the common logic for initializing the EditorHandle and PANIC_DIALOG_MESSAGE_CALLBACK into a separate helper function that both create functions can call.

frontend/wasm/src/lib.rs (84-90)

medium

There's a redundant string allocation here. The panic_hook function already converts the panic info to a String. This function then receives it as a &str and converts it back to a String using to_string(), which is inefficient.

To avoid this extra allocation, you could change this function's signature to accept a &String and then use clone() to create the FrontendMessage. The call site in panic_hook would not need to change. The same improvement applies to the send_panic_dialog function.

fn send_panic_dialog_via_callback(panic_info: &String) -> bool {
	let message = FrontendMessage::DisplayDialogPanic { panic_info: panic_info.clone() };
	let message_type = message.to_discriminant().local_name();
	let Ok(message_data) = serde_wasm_bindgen::to_value(&message) else {
		log::error!("Failed to serialize crash dialog panic message");
		return false;
	};

@JustJ01 JustJ01 marked this pull request as ready for review February 18, 2026 21:22
@TrueDoctor
Copy link
Member

@JustJ01 the suggested allocation removal is bs. Calling .clone() on a &String will still allocate and basically compiles to the same code as would &str. But you could try taking an owned String as the argument to the function (instead of the &String) to actually avoid the allocation

@JustJ01
Copy link
Contributor Author

JustJ01 commented Feb 18, 2026

@JustJ01 the suggested allocation removal is bs. Calling .clone() on a &String will still allocate and basically compiles to the same code as would &str. But you could try taking an owned String as the argument to the function (instead of the &String) to actually avoid the allocation

@TrueDoctor I have made the changes

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.

Panics originating in a requestAnimationFrame loop silently freeze the editor

2 participants

Comments