Fix #306: end the stream when the console suspends - #318
Conversation
|
Hardware result, on a V1 console with Atmosphere, running in title takeover mode ( The hang is gone. Sleep during an active stream, then wake, previously gave a dead screen needing a 15 second power hold and a fresh payload injection. With this change the console wakes, the display comes back after a brief white flash while the display stack reinitialises, and the app stays responsive and exits cleanly. The overlay confirmed the path runs, in order: Also checked the case this could plausibly have regressed: pressing HOME mid stream, waiting, and returning. The stream stays up and resumes, so the behaviour One thing this does not fix, which was previously invisible because the console bricked first: after a real sleep the stream does not recover. Wifi is off while asleep, so the connection dies, and That looks like a separate issue in the connection lifecycle rather than something to fold in here, so I have left this PR scoped to the hang. I am instrumenting a build to find out whether |
|
Correction to my previous comment, plus the direct evidence for the root cause. I was wrong that the stream does not recover. It does. With a file log attached, a real sleep and wake gives: The reconnect in The evidence for the stale mapping. The same log confirms the decoder surfaces really are reallocated across a sleep, which is what makes the invalidation in this PR load bearing rather than merely harmless. nvmap handles cached by On resume: and the handles the renderer then maps after reconnecting: A completely disjoint set. Every mapping cached before the sleep was pointing at decoder memory that no longer existed in that form. Without the invalidation, The instrumentation used to produce this is a local diagnostic build, not part of this PR. |
|
What happens if you remove |
|
I tested it out. I built three versions: master with that line deleted, master untouched as a control, and this PR's branch unmodified. I used the same commit for the first two, so the only difference is just that one line. With the line deleted, sleep and resume still crashes. I got a solid orange screen, the console hung, and I had to hold down power to get out of it. Pressing Home mid-stream also kills the stream, and doing it a second time panicked Atmosphere with With the untouched master on the same commit: sleep and resume drops the stream, but the app stays up and relaunches fine, and pressing Home works 3/3 time. This PR's branch survives sleep and resume 3/3 with the controls still working afterwards. So it doesn't fix the crash, and it basically turns a recoverable stream drop into a hung console. One caveat: stock master degraded gracefully for me rather than hard-crashing the way #306 describes. So, this is really just measuring the effect of removing the call, not a reproduction of the original report. Also, the call isn't even in this repo. It's in the borealis submodule at |
|
Why not pause the stream and return to the games menu upon console sleep? This should avoid all "stream surviving sleep mode" issues, and is consistent with behavior on Android (though a bit less convenient than fully surviving sleep, but even then it's a "good enough" solution even if a better fix gets added in the future). |
Moonlight on Android ends the session when the app stops: Game.onStop() calls stopConnection() then finish(), so sleeping the phone mid stream drops you back to the host list. This does the same on Switch, where losing applet focus means the console sleeping or the HOME menu taking over. On focus loss it drops input and calls the existing terminate(false), so the stream ends and the view falls back to the host list. terminateApp is false, so only the stream stops; whatever is running on the host keeps running and can be resumed by reconnecting. Issue XITRIX#306 is a hung console after waking, and it lives in the window where the OS tears the graphics and network services down underneath a process that keeps running. Ending the stream removes the state rather than trying to repair it: there is nothing stale left to get wrong. Tested on hardware, three sleep cycles plus HOME: the stream ends, the host list appears, reconnecting works, no hang. Co-Authored-By: Claude <noreply@anthropic.com>
6f5fa71 to
6bd4dfe
Compare
quadseven
changed the title
|
Swapped this PR over to your suggestion. I built both and tested them on hardware first, three sleeps each plus home. Parity version ends the stream, lands on the host list, reconnects fine. The old approach kept the stream alive with controls working. Neither hangs, so reliability was a wash and it came down to which is less to go wrong later. This one reuses the existing Old approach is on my fork as |
This comment was marked as outdated.
This comment was marked as outdated.
Tried copying the file over the installed filename. Now it sometimes exits properly and sometimes crashes upon wake. I've managed to reproduce the crash via home screen, which should be easier to debug than sleep (since the latter disconnects gdb). |
The bug is that you call If I breakpoint It appears the change made is wildly concurrency-unsafe. Out of curiosity can you push your old code to a different branch for me to test? |
Reported with a gdb backtrace by nyanpasu64: SIGSEGV in MoonlightSession::stop with this=0x30, reached from appletCallHook -> Event<bool>::fire -> StreamingView::terminate.
The cause is in borealis' Event::fire:
for (Callback cb : this->callbacks)
cb(args...);
It iterates the callback list. terminate() calls dismiss(), which pops this view and runs its destructor, and the destructor unsubscribes. Doing that from inside the callback mutates the list fire() is walking, so the iteration continues over a destroyed object. That is the dangling this.
The callback now only records the intent and draw() acts on it, where the view is known alive and nothing is iterating the event. When the focus loss was a console sleep this runs on the first frame after waking, which is also the first moment the graphics service is back, so the teardown no longer happens while the OS is mid suspend.
Also null guards session in draw().
This does not address the separate async-start race in the second backtrace, where a MoonlightSession start callback runs on a thread pool thread while the view is going away. That one predates this change.
Co-Authored-By: Claude <noreply@anthropic.com>
|
Nice catch, thanks for checking it out. The bug was that I was tearing the view down from inside the focus callback. borealis loops over its callback list when it fires the event, and my code was destroying the view halfway through that loop, so it kept going over something that no longer existed. Hence the garbage pointer. Fixed by not doing anything destructive in the callback. It just sets a flag now and the next frame does the actual teardown, where nothing is mid-loop. When it’s a sleep that ends up happening on the first frame after waking, which is also nicer because it’s not tearing things down while the console is still going under. Retested here, pressed home a bunch of times and did several sleeps, no crashes. I’m on release libnx though, not your beta, so it’d be good to know if it holds up on yours. And yes, old renderer version is up as archive/306-renderer-invalidation on my fork. It’s straight off current master so it should build as-is. |
|
I did notice one annoyance: I'd get a wall of "Input queue reached maximum size limit" after going to the home menu and returning to the app. Redirecting stdout to the PC using: registerDeepLinkHandler();
nxlinkStdio();followed by: make Moonlight.nro -j$(nproc) && nxlink -a 192.168.0.67 Moonlight.nro --serverproduced the following output: Annoyingly there's 16 different occurrences of "Input queue reached maximum size limit" in the code. Picking a random one with streaming inputs,
(Also out of curiosity how much of your analysis was by hand, and how much of the code authorship was by hand, rather than LLM-written artifacts? I'm doing my debugging largely on my own.) |
The focus subscription was created in onFocusGained, which runs every time the view regains focus, including when an overlay closes. The destructor removes one subscription, so every extra focus cycle left a callback registered on a global event pointing at freed memory once the view went away. Reported with a backtrace showing the dangling this. Deferring the teardown to draw() did not fix that. The leaked callbacks still ran; they just wrote a flag into a destroyed object instead of calling terminate on it. Subscribe once, in the constructor, immediately after session exists. Log focusSubscriptionCount on both sides: one streaming view exists at a time, so it is 1 while streaming and 0 otherwise, and anything else is this bug announcing itself instead of staying silent. brls::Event keeps its callback list private and offers no count of its own. Also disable input before dropping it on focus loss, which is where the wall of input queue limit messages came from, and drop the null check on session, which is assigned in the constructor and never cleared. Co-Authored-By: Claude <noreply@anthropic.com>
|
Thoughts:
All in all the crashing and non-crashing paths have noticeable differences, but I don't know what's responsible for the remaining crash, and haven't started selectively executing or stubbing parts of the code to narrow down the culprit.
Caveats:
|
|
master...nyanpasu64:Moonlight-Switch:fork#files_bucket it works. more or less.
|
|
Still working on this. Yes, the code is mostly LLM written. I'm not a Switch dev, I hacked this thing for the first time a few weeks ago so I could stream Sunshine off my PC instead of buying a Steam Deck, it started hanging whenever I slept it mid stream, and I threw an LLM at it and started testing on hardware. The testing is mine and I'm not going to argue about code I don't fully understand. After you mentioned the repeated ClInternalConnectionTerminated, I put spans with thread ids around the focus change and the termination callback on my fork, written to the SD card as each one completes so a hang still leaves them behind. It crashed again last night and caught this: So they overlap, on different threads, rather than just landing near each other. connection_terminated is still inside its reconnect attempt, error code 115, when the applet regains focus on the main thread and the resume path starts running underneath it. Correcting something I had in this comment earlier: I said the log stopped mid word about 40ms later, partway through "Stopping control stream", and read that as roughly where it died. That isn't right. borealis only fflush()es per line under The spans are the trustworthy part, since each is written and flushed as it completes. And they say connection_terminated closed normally at 100.20 ms, so it returned and whatever killed the process ran after it. No crash report and no fatal report on the card either. There are two ERPT records four seconds later but they're routine noise, the same pair this console writes every few minutes with AbortFlag false. That sent me reading moonlight-common-c rather than our code, and the part I hadn't understood is that LiStopConnection ends up called from two threads. connection_terminated calls it on the AsyncTerm thread to reconnect, and StreamingView::terminate calls it on the main thread through MoonlightSession::stop. stop() only guards on m_stop_requested, and the reconnect path deliberately doesn't set that flag, so nothing excludes the two. The stage ladder in LiStopConnection has no lock. Calling LiStopConnection from inside the callback on its own looks fine, the AsyncTerm thread in Connection.c exists so that doesn't deadlock. Restarting afterwards is the part I can't find support for: Limelight.h says LiStartConnection isn't thread safe, and it resets ConnectionInterrupted and alreadyTerminated, which stopControlStream and the ENet loop read to know when to stop. moonlight-qt looks like it deliberately does the opposite. Its clConnectionTerminated doesn't call LiStopConnection at all, it pushes SDL_QUIT and returns, the main thread unwinds, and LiStopConnection runs once from there with a semaphore held until it returns so a restart can't overlap a teardown. Which I think is what you've already arrived at with mainThreadRestart. So sleep probably isn't the trigger, it's the termination callback and the resume being live at the same time, which would be why pulling the network gets you there too. Leaving this PR open for XITRIX to decide on, it's their repo and their call. If you want to open yours alongside it, please do, better they have both to pick from. If it's useful having someone with hardware who'll run whatever build you want and send back traces like that, I'm around. |
|
Never reproduced the crash on gdb, but managed to recconstruct the call stack from atmosphere/crash_reports/...log. /opt/devkitpro/devkitA64/bin/aarch64-none-elf-addr2line -e Moonlight.elf -fpC 0x9a7a0 0xac224 0x23f93c 0x23f04c 0x23a998 0x26b4b4 0x24407c 0x126304 0x2440b4 0x23e998 0x263060Don't have time to look into it now; my current problem is that the frame pacing added to master increases input latency to the point mouse movement is difficult to control, and I need to figure out how it works and how to wrangle it into a useful state. |
|
Tested this fix on FW 21.1.0 + Atmosphere 1.11.2 (emuMMC), as a custom NRO build. Previously: sleeping the console mid-stream caused a hard orange-screen hang on wake (had to hard power off). With this PR applied: auto-sleep, HOME-menu sleep, and hold-HOME popup sleep all end the stream cleanly and return to the app selection screen relaunching reconnects instantly. Zero crashes across a full day of testing. |
3 participants
Size: XS
Why
#306 is a hung console after waking from sleep mid stream. Orange screen, unresponsive, hard power off to recover.
The hang lives in the window where the OS tears the graphics and network services down underneath a process that keeps running. This ends the stream at that point instead of trying to carry decoder and GPU state through it, so there is no stale state left to get wrong.
This matches Moonlight on Android, where
Game.onStop()callsstopConnection()thenfinish(): sleeping the phone mid stream ends the session and returns you to the host list. Suggested in review by @nyanpasu64, and after building and testing both approaches on hardware I agree it is the better one to ship.What changed
On losing applet focus, drop input and call the existing
terminate(false).terminateAppis false, so only the stream ends. Whatever is running on the host keeps running and can be resumed by reconnecting.Acceptance criteria
Test plan
Tested on hardware, Switch on Atmosphere, streaming a game from a PC:
I also built and tested the alternative, which keeps the session alive across the suspend and repairs the GPU frame mappings on the first resumed frame. That also survived three sleeps with no hang, so both are reliable. It is on my fork as
archive/306-renderer-invalidationif it is ever wanted.Out of scope