Repository navigation
Fix the app hanging on Connect to Server - #23
Merged
Merged
Conversation
v0.2.9's Link::start locked the playout to hand it the waveform channel, but the app calls it from the UI thread with that lock already held, so connecting deadlocked the app (the server saw a normal sign-in). The mirror now gets its wave channel from Playout::wave_requests before the swap, and Link::start takes the receiver without locking. The server file picker had the same re-lock while connected; it now reads the library folders from the playout it is given. Adds a test that connects with the playout locked, as the app does. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0121MtpepbszTWK1oCTggoPk
![mergeguards[bot]](https://avatars.githubusercontent.com/u/280023544?s=60&v=4){kind=small}
There was a problem hiding this comment.
MergeGuard review
Risk: 🔴 60/100 — risky
Findings: 🔴 1 high
Open Files changed for 1 inline review thread on the diff.
💡 Use the Fix All MergeGuard Findings checkbox comment on this PR to apply auto-fixes in batch, or reply
@mergeguards fixon any inline finding.
⚠️ This PR may benefit from a deep scan — consider triggering@mergeguards deep-scan.
Reviewed by MergeGuard AI
MergeGuard agent actionsFix all findingsAll issues (1)
Apply auto-fixes for the findings above on this branch:
Or reply with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by epell · project thread
What broke
In v0.2.9 the desktop app freezes ("crashes") as soon as it connects to a server, while the server logs a normal sign-in.
Link::startgaineds.core.lock().set_wave_source(..)(track editor waveforms from the server, #21). The app callsconnect→Link::startfrom the UI thread with the playout already locked (app.rs, thearc.lock()beforeserver_windows/remote_sign_in).parking_lot::Mutexisn't re-entrant, so the UI thread deadlocks on itself. The existing server tests callLink::startwithout holding the lock, so they didn't catch it.Fix
Playout::set_wave_source(tx)→Playout::wave_requests() -> Receiver<PathBuf>;connectcalls it on the mirror before the swap.Link::starttakes thewavesreceiver and never lockscore(documented).server_picker_windowdidself.core.lock()underserver_windows, so opening a file dialog while connected would also hang): it now reads the library folders from thecoreit's given.Test
server::tests::connecting_with_the_playout_lockedconnects exactly like the app (lock held,mirror_of, swap,Link::start) and fails within 10 s if it hangs. Verified it fails on the old code and passes now.cargo fmt,cargo +1.99.0 clippy --all-targets -- -D warnings,cargo test(both binaries) all pass.CHANGELOG (for v0.2.10)
Fixed
Generated by Claude Code