Fixes #5016 - Fix deadlock in EventIPCTransport.DispatchWailsEvent holding RLock during InvokeSync

[wayneforrest]

Description

Potential deadlock in EventIPCTransport.DispatchWailsEvent lock ordering issue.
See details in #5106

Fixes #5016

Type of change

Please select the option that is relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using wails doctor.

• Windows
• macOS
• Linux

If you checked Linux, please specify the distro and version.

Test Configuration

Please paste the output of wails doctor. If you are unable to run this command, please describe your environment in as much detail as possible.

Checklist:

• I have updated website/src/pages/changelog.mdx with details of this PR
• My code follows the general coding style of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

Summary by CodeRabbit

• Bug Fixes

  • Resolved a deadlock during event dispatch that could block the app, improving stability.

• Refactor

  • Reduced lock contention during event processing by taking a snapshot of targets before dispatch, improving responsiveness and performance.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ed999c50-f205-40b4-ba62-0b2bdf7b9643

📥 Commits

Reviewing files that changed from the base of the PR and between 838b00a and e65d2da.

📒 Files selected for processing (1)

• v3/UNRELEASED_CHANGELOG.md

✅ Files skipped from review due to trivial changes (1)

• v3/UNRELEASED_CHANGELOG.md

---

Walkthrough

DispatchWailsEvent now snapshots app.windows under app.windowsLock.RLock() and releases the lock before iterating and calling window.DispatchWailsEvent(event). Cancellation checks remain but are performed while iterating over the snapshot rather than while holding the lock.

Changes

Cohort / File(s)	Summary
IPC Event Dispatch Locking v3/pkg/application/transport_event_ipc.go	Snapshot app.windows under app.windowsLock.RLock() then release the lock before dispatching events to each window; preserves cancellation checks during iteration.
Changelog v3/UNRELEASED_CHANGELOG.md	Adds a "Fixed" entry noting resolution of a deadlock in EventIPCTransport.DispatchWailsEvent where an RLock was previously held during InvokeSync.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• [wails3] deadlock - wails v3 alpha.74 #5106: Matches the deadlock fix in EventIPCTransport.DispatchWailsEvent by snapshotting windows under RLock and releasing before dispatch calls.

Suggested labels

Bug, Documentation, go, v3-alpha

Suggested reviewers

• leaanthony

Poem

🐰 I snapped the list, then set it free,
No held‑locks clogging event spree.
Through quiet hops and tiny cheer,
Events now travel swift and clear. ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: fixing a deadlock in EventIPCTransport.DispatchWailsEvent by addressing lock ordering issues with RLock during InvokeSync.
Description check	✅ Passed	The PR description covers the bug fix nature and references the relevant issues (#5016 and #5106), though it lacks test configuration details and most checklist items remain unchecked.
Linked Issues check	✅ Passed	The PR fixes a deadlock in DispatchWailsEvent that was preventing frameless window minimization on macOS (#5016), which is the stated objective of the linked issue.
Out of Scope Changes check	✅ Passed	All changes are in-scope: modifications to transport_event_ipc.go address the deadlock, and the changelog entry documents the fix as required.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

v3/pkg/application/transport_event_ipc.go (1)

22-27: Please add a regression test for lock inversion deadlock.

This path is concurrency-sensitive; a targeted test that dispatches while windows are added/removed would help prevent regressions.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@v3/pkg/application/transport_event_ipc.go` around lines 22 - 27, Add a
concurrent regression test that reproduces the lock-inversion scenario by
repeatedly calling DispatchWailsEvent while other goroutines concurrently
add/remove entries from the windows collection; specifically create a test
(e.g., TestDispatchWhileModifyingWindows) that spins up one goroutine to loop
window.DispatchWailsEvent(event) and another to concurrently add and remove
windows, coordinate start/stop with sync.WaitGroup or channels, enforce a
timeout/context deadline to fail the test on deadlock, and assert progress
(e.g., event delivered or loop iterations completed) to detect hangs; target the
code paths around windows, DispatchWailsEvent and event.IsCancelled to ensure
the concurrency-sensitive path is covered under the race detector.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@v3/pkg/application/transport_event_ipc.go`:
- Around line 22-27: Add a concurrent regression test that reproduces the
lock-inversion scenario by repeatedly calling DispatchWailsEvent while other
goroutines concurrently add/remove entries from the windows collection;
specifically create a test (e.g., TestDispatchWhileModifyingWindows) that spins
up one goroutine to loop window.DispatchWailsEvent(event) and another to
concurrently add and remove windows, coordinate start/stop with sync.WaitGroup
or channels, enforce a timeout/context deadline to fail the test on deadlock,
and assert progress (e.g., event delivered or loop iterations completed) to
detect hangs; target the code paths around windows, DispatchWailsEvent and
event.IsCancelled to ensure the concurrency-sensitive path is covered under the
race detector.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2e8d59ec-2082-44c3-99d0-7767a809b2cd

📥 Commits

Reviewing files that changed from the base of the PR and between bb4fbf9 and 838b00a.

📒 Files selected for processing (1)

• v3/pkg/application/transport_event_ipc.go