Skip to content

fix: prevent freeze on double right-click in directory background#334

Open
lidaixingchen wants to merge 2 commits intostd-microblock:masterfrom
lidaixingchen:fix/double-right-click-freeze
Open

fix: prevent freeze on double right-click in directory background#334
lidaixingchen wants to merge 2 commits intostd-microblock:masterfrom
lidaixingchen:fix/double-right-click-freeze

Conversation

@lidaixingchen
Copy link
Copy Markdown
Contributor

Problem

When performing two consecutive right-clicks on the directory background area, the file explorer freezes.

Root Cause

Three interacting issues identified through debug.log analysis:

  1. Single-threaded renderer_thread deadlock: The second track_popup_menu call gets queued behind the first one's event loop, causing the main thread to hang in nested wait_with_msgloop().
  2. Race condition on menu_callbacks_js: The file watcher thread clears and repopulates menu_callbacks_js without synchronization, while the renderer thread iterates over it concurrently.
  3. Excessive script reloading: Multiple file changes trigger cascading reloads without debouncing, causing init_known_strings to block for 200-700ms each time.

Fixes

  • Add re-entrancy protection to track_popup_menu: when a menu is already showing, close it first and wait for completion before creating a new one.
  • Add std::shared_mutex for menu_callbacks_js: write lock for clear/add/remove operations, read lock for iteration.
  • Add 500ms debounce delay for script reloading to batch file changes.

Changed Files

  • src/shell/contextmenu/hooks.cc - re-entrancy protection
  • src/shell/contextmenu/menu_render.cc - shared_lock read protection
  • src/shell/script/binding_types.hpp - declare menu_callbacks_js_mutex
  • src/shell/script/binding_types.cc - define mutex, add write lock
  • src/shell/script/script.cc - write lock protection and debounce

When performing two consecutive right-clicks on the directory background
area, the file explorer freezes due to three interacting issues:

1. Single-threaded renderer_thread deadlock: The second track_popup_menu
   call gets queued behind the first one's event loop, causing the main
   thread to hang in nested wait_with_msgloop().

2. Race condition on menu_callbacks_js: The file watcher thread clears
   and repopulates menu_callbacks_js without synchronization, while the
   renderer thread iterates over it concurrently.

3. Excessive script reloading: Multiple file changes trigger cascading
   reloads without debouncing, causing init_known_strings to block for
   200-700ms each time.

Fixes:
- Add re-entrancy protection to track_popup_menu: when a menu is already
  showing, close it first and wait for completion before creating a new one.
- Add std::shared_mutex for menu_callbacks_js: write lock for clear/add/
  remove operations, read lock for iteration.
- Add 500ms debounce delay for script reloading to batch file changes.
Copilot AI review requested due to automatic review settings April 11, 2026 19:59
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a file-explorer freeze triggered by consecutive right-clicks on the directory background by addressing re-entrancy in context menu display, synchronizing JS menu listener access, and debouncing script reloads.

Changes:

  • Add re-entrancy handling in track_popup_menu to close an existing menu before showing a new one.
  • Protect menu_callbacks_js with a std::shared_mutex (writers in bindings/reload; readers in menu rendering).
  • Debounce JS script reloads to batch rapid filesystem change events.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/shell/contextmenu/hooks.cc Adds re-entrancy detection/close-and-wait logic around track_popup_menu.
src/shell/contextmenu/menu_render.cc Wraps iteration of JS menu listeners with a shared lock.
src/shell/script/binding_types.hpp Declares a global menu_callbacks_js_mutex for synchronizing listener access.
src/shell/script/binding_types.cc Defines the mutex and adds write-locking for add/remove listener operations.
src/shell/script/script.cc Adds write-locking for clearing listeners during reload and introduces reload debouncing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Add missing #include <condition_variable> in hooks.cc
- Fix re-entrancy race: use exchange(true) to set is_menu_showing
  synchronously in track_popup_menu, preventing the window where
  a second call could slip through before the renderer thread sets the flag
- Fix potential deadlock in menu_render.cc: copy menu_callbacks_js
  under shared_lock, release lock, then invoke callbacks on the snapshot
- Add missing #include <mutex> in script.cc
- Fix data race on has_update/last_change_time: protect with
  file_change_mutex in both FileWatch callback and polling loop
@std-microblock
Copy link
Copy Markdown
Owner

我不太能复现这个问题呢 🤔

@lidaixingchen
Copy link
Copy Markdown
Contributor Author

添加保护总是没错的。🤔 #324 #330

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.

3 participants