Skip to content

Fix EDT freezes when opening embedded DevTools and on theme change #8477

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

Fix EDT freezes when opening embedded DevTools and on theme change #8477

wants to merge 3 commits into from

Conversation

Copy link

Copilot AI commented Aug 20, 2025 •
edited by AlexV525
Loading

Summary

  • Fix EDT freezes caused by:
    • Synchronous JxBrowser Engine initialization when opening embedded DevTools.
    • Blocking ExecutorService lifecycle on theme change (JDK 21 ExecutorService.close() awaits termination).

Changes

  • EmbeddedJxBrowser.openEmbeddedTab:
    • Avoid initializing JxBrowser Engine on the EDT; show “installation in progress” UI until async init completes.
    • This removes the fallback compareAndSet(null, getEngine()) path that could call Engine.newInstance(...) on EDT and freeze it.
  • FlutterInitializer.sendThemeChangedEvent:
    • Use shared AppScheduledExecutor (AppExecutorUtil) instead of creating/closing a new executor per event.
    • Remove try-with-resources that triggers ExecutorService.close() on EDT with JDK 21.

Notes

  • Added comments explaining why Engine initialization must not occur on EDT and how JDK 21 changed ExecutorService.close().
  • Included a lightweight regression test to ensure openEmbeddedTab returns quickly on EDT when the engine is not yet available.

Fixes

Testing

  • Unit: EmbeddedJxBrowserTest.openEmbeddedTab_returnsQuicklyOnEdtWhenEngineMissing ensures the call returns promptly on EDT (no sync engine init).
  • Manual:
    • Switch IDE themes repeatedly; no UI freeze.
    • Open DevTools (embedded) after clean startup and after JxBrowser installation completes; no EDT freeze, proper “installing...” UX during setup.

@google-cla
Copy link

google-cla bot commented Aug 20, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@AlexV525 AlexV525 added the 🤖 AI-assisted For PRs with significant code generated by AI label Aug 20, 2025
Copilot AI changed the title [WIP] ⚡️ Prevent EDT freeze on theme change by using a shared scheduler Fix EDT freeze in sendThemeChangedEvent by using shared ScheduledExecutorService Aug 20, 2025
Copilot AI requested a review from AlexV525 August 20, 2025 02:27
@AlexV525 AlexV525 changed the title Fix EDT freeze in sendThemeChangedEvent by using shared ScheduledExecutorService Fix EDT freezes when opening embedded DevTools and on theme change Aug 20, 2025
@AlexV525 AlexV525 force-pushed the copilot/fix-9420cc20-f930-4082-8323-e7d16fbf138c branch from d346a5a to 9c6ed7f Compare August 20, 2025 03:13
@AlexV525 AlexV525 closed this Aug 20, 2025
@AlexV525
Copy link
Member

AlexV525 commented Aug 20, 2025

Apparently, Google's CLA is not ready for the Copilot yet. :)

@AlexV525 AlexV525 mentioned this pull request Aug 20, 2025
Merged
@jwren jwren added this to the M88 milestone Aug 21, 2025
jwren pushed a commit that referenced this pull request Aug 23, 2025
@AlexV525
Fix DTD freezes when opening projects, and EDT freezes when the theme…
25f8e08 
… changed and opening embedded DevTools (#8479)

*(originally with
#8477

Summary
- Fix frequent “Timed out waiting for DTD websocket to start” errors
when opening projects.
- Fix EDT freezes caused by:
- Synchronous JxBrowser Engine initialization when opening embedded
DevTools.
- Blocking ExecutorService lifecycle on theme change (JDK 21
ExecutorService.close() awaits termination).

Changes
- DtdUtils
- Refactor readyDtdService to be truly asynchronous using a shared
ScheduledExecutorService for periodic polling (no blocking).
- De-duplicate and cache the in-flight Future per Project to avoid
redundant polling under concurrency.
- Preserve an overall timeout (~20s by default). On timeout or error,
complete exceptionally and clear the cache to allow subsequent retries.
- EmbeddedJxBrowser.openEmbeddedTab:
- Avoid initializing JxBrowser Engine on the EDT; show “installation in
progress” UI until async init completes.
- This removes the fallback compareAndSet(null, getEngine()) path that
could call Engine.newInstance(...) on EDT and freeze it.
- FlutterInitializer.sendThemeChangedEvent:
- Send themeChanged asynchronously: remove .get() blocking; use a shared
scheduler with debounce.
- If DTD becomes ready later, the event is sent then; if it times out,
gracefully skip with debug-level logging (no startup errors).
- Use shared AppScheduledExecutor (AppExecutorUtil) instead of
creating/closing a new executor per event.
- Remove try-with-resources that triggers ExecutorService.close() on EDT
with JDK 21.

Notes
- Added comments explaining why Engine initialization must not occur on
EDT and how JDK 21 changed ExecutorService.close().

Fixes
- Fixes #7794
- Fixes #8394
- Fixes #8478

Testing
- Manual:
  - Open Flutter projects; no error logs about DTD timeout.
  - Switch IDE themes repeatedly; no UI freeze.
- Open DevTools (embedded) after clean startup and after JxBrowser
installation completes; no EDT freeze, proper “installing...” UX during
setup.
@AlexV525 AlexV525 removed this from the M88 milestone Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexV525 AlexV525 Awaiting requested review from AlexV525

Assignees

@AlexV525 AlexV525

Copilot code review Copilot

Labels

🤖 AI-assisted For PRs with significant code generated by AI

Projects

None yet

Milestone

No milestone

2 participants

@AlexV525 @jwren