Repository navigation
fix(linux): stop a new stream from crashing streams already encoding with CUDA - #6014
Conversation
kriansa
changed the title
Every stream that starts calls cuda::init(), which reloads the table of CUDA driver functions that every encode device shares. The loader empties the table before filling it in again, so a stream already encoding through it can call a null function pointer and take the whole process down. On Linux with NVENC this crashes Sunshine at random when a second client connects while another one is streaming. cuda::init() now hands the table and the loader to an overload, the way device_pci_bus_id() takes its driver operations, so tests can use a fake loader and need no GPU. One test checks that a second init() does not load the table again; it fails until the next commit. The other checks that a load that failed is retried on the next call.
cuda::init() reloaded the shared CUDA function table on every call, and every new stream calls it. Reloading empties the table before filling it again, so a stream already encoding through it could call a null function pointer, crashing Sunshine when a second client connected. Load the table only when it is not loaded yet, and keep it for the life of the process. Later calls only run cuInit again, so a first load that failed is still retried. A mutex stops streams that start at the same time from loading it concurrently.
ffe05c7 to
792ff31
Compare
ReenigneArcher
left a comment
ReenigneArcher
left a comment
There was a problem hiding this comment.
The load-once change addresses a race that is still present on current master (4cce5bf6). Requesting changes because the new injected initialization path can dereference the unrelated global table when CUDA initialization fails.
Please extend tests/unit/platform/linux/test_cuda.cpp to cover both remaining behaviors:
- Make the injected
cuInitfail once, supply fake error-name/error-string callbacks, and assert failure is reported through those callbacks. Retry successfully and assert the same table is retained, the loader ran once, andcuInitran twice. - Start multiple callers together with a gated/delayed fake loader and thread-safe counters; assert every caller succeeds, the table is loaded once, and
cuInitruns once per caller. The two existing sequential cases do not detect removal of the mutex.
Run these in a CUDA-enabled Linux build with cmake-build-debug/tests/test_sunshine --gtest_filter='CudaInitTest.*' (adjust the build-directory name).
GitHub reports confirmed merge conflicts, also reproduced locally in src/platform/linux/cuda.cpp and tests/unit/platform/linux/test_cuda.cpp. Please rebase onto master and resolve them before merging, retaining the table-aware error-checking helper introduced by #6010.
Validation: git diff --check and clang-format 23.1.0 dry-run checks passed for the changed files. A macOS harness using the exact PR initialization/check bodies and ASan/UBSan passed sequential load, loader retry, and 25 rounds of 16 concurrent callers; injected initialization failure triggered a null-pointer diagnostic. The table-aware correction passed all four cases. These are isolated checks, not a run of the repository's Linux test executable or GPU streaming validation.
Hosted verification remains limited by workflows awaiting maintainer approval. I inspected the failed GH-Pages documentation build: it timed out waiting for the absent Jekyll workflow, rather than reporting a source/documentation error.
Review completed by Codex | Model: gpt-6.1-sol | Thinking: xhigh
# Conflicts: # src/platform/linux/cuda.cpp # tests/unit/platform/linux/test_cuda.cpp
init(table, load) checked the result of cuInit with CU_CHECK, which looks up the error's name and description through the global table rather than the one passed in. In production they are the same table, but with any other table a failing cuInit dereferences the global one, which may not be loaded at all, instead of returning -1. Use the table-aware check() from LizardByte#6010 instead.
Two cases the existing init() tests miss, both raised in review. A failing cuInit must be reported through the error callbacks of the table init() was given, and the retry must reuse that table: one load, two cuInit calls. Callers starting together must load the table once between them. Eight of them start at once against a slow fake loader, whose first load waits until a second caller reaches the loader, or 200 ms at most. The sequential tests still pass without the mutex; this one fails every time. The loader hands out a shared table and frees nothing, so a regression shows up as a wrong count rather than a double-free crash. The fakes move into one shared state with atomic counters, since the loader is a plain function pointer and the callers run on several threads.
SonarCloud flags function pointers used as parameters (cpp:S5205), which failed the quality gate on init(table, load). Take the loader as a std::function by const reference instead, as the other injected operations in cuda.h already do. Callers don't change: plain functions and lambdas convert to it.
|
Thanks for the thorough review. I merged master ( About the tests:
Results (
Full suite: 1022 passed, 0 failed, 22 skipped. End to end on current master, on the real GPU:
clang-format 23.1.0 and |
|
Bundle ReportChanges will decrease total bundle size by 1.48kB (-0.04%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: sunshine-esmAssets Changed:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6014 +/- ##
==========================================
- Coverage 49.34% 48.49% -0.85%
==========================================
Files 143 141 -2
Lines 31292 30963 -329
Branches 13350 13061 -289
==========================================
- Hits 15441 15017 -424
+ Misses 14538 13429 -1109
- Partials 1313 2517 +1204
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 59 files with indirect coverage changes Continue to review full report in Codecov by Harness.
|
Screenshot ComparisonPR #6014 screenshots vs Matrix:
|
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Matrix: AppImage-x86_64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Matrix: Windows-AMD64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
|
sunshine_tray_menu.png |
 |
|
sunshine_tray_pairing_request.png |
 |
|
sunshine_tray_paused.png |
 |
|
sunshine_tray_stopped.png |
 |
|
sunshine_tray_streaming.png |
 |
|
sunshine_tray_tooltip.png |
 |
Matrix: Windows-ARM64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Matrix: macOS-arm64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Matrix: macOS-x86_64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Description
Sunshine randomly crashes when a second client connects while another one is already streaming. It doesn't happen on every connect (about one in three in my case), but when it does, every client's stream drops at once. Seen on Linux with NVENC and wlr capture (headless labwc, NVIDIA 615.x). The only trace is in the kernel log:
The cause is that
cuda::init()reloads the shared CUDA function table (cdf) on every call, and every new stream calls it when it creates its encode device.cuda_load_functions()frees the current table, installs a zeroed one, and only then openslibcuda.so.1and fills the entries in. A stream that is already encoding calls throughcdfon every frame. If it does so inside that window, it calls a null function pointer and the whole process dies.Backtrace from the core dump (v2026.914.233613;
masterhas the sameinit()):On current
master(after #6010) the running stream crashes the same way, now in the GL→CUDA handoff:This change loads the table once and keeps it for the life of the process. Later calls only run
cuInit(0)again, so a first load that failed is still retried. A mutex stops streams that start at the same time from loading it concurrently. To test this without a GPU,cuda::init()now hands the table and the loader to aninit(table, load)overload, the same waydevice_pci_bus_id()takes its driver operations. The overload reports a failingcuInitthrough the table it was given, using the table-awarecheck()from #6010.Screenshot
Issues Fixed or Closed
Roadmap Issues
Testing
I first made the crash reproducible on demand, so it happened on every attempt instead of about one connect in three. Then I applied the fix, rebuilt Sunshine, and ran it on the same machine. The end-to-end tests with the fix were clean: no crashes, and both streams kept running. The crash happened every time before the fix and never after it, which confirms the fix resolves it. The details are below.
Regression tests in
tests/unit/platform/linux/test_cuda.cpp. They use a fake loader, so they need no GPU and run in CI:LoadsTheFunctionTableOnlyOnce: a secondinit()must not load the table again.ReportsACuInitFailureThroughTheTableAndRetriesItWithTheSameTable: a failingcuInitis reported through the fake error callbacks of the table given, and the retry succeeds with the same table, one load and twocuInitcalls.CallersStartingTogetherLoadTheTableOnce: 8 callers start together against a slow fake loader. All succeed, the table is loaded once, andcuInitruns once per caller. The first load waits until a second caller reaches the loader, or 200 ms at most, so the test costs about 200 ms when it passes.RetriesLoadingAfterAFailure: a load that failed is retried on the next call.The first three fail on a build with these tests but without the fix, and pass with it, in 20 out of 20 repeats each. The fourth passes on both. As a check of the third test, removing only the mutex (keeping the load-once check) made it fail in 50 out of 50 repeats while the other three still passed.
Full
test_sunshinesuite, in a Fedora 44 container: 1019 passed and 3 failed (the tests above) without the fix, and 1022 passed with it. 22 tests were skipped in both runs.Against the real driver (Fedora 44, RTX 3050, driver 615.71.10): a manual check, not part of this PR, intercepted
dlopen("libcuda.so.1")during a secondinit(). Without the fix, the entry every frame conversion calls first (cuGraphicsMapResources) was null in 30 out of 30 runs.End to end: headless labwc, wlr capture, NVENC, with one client streaming continuously. A preloaded shim delayed
dlopen("libcuda.so.1")by 100 ms to widen the window:Repeated after merging current
master(4cce5bf6):Built with
BUILD_WERROR=ON(gcc 15, CUDA 13.1.1). clang-format 23.1.0 andgit diff --checkreport nothing, and the docs build passes. Not run: CI, Windows and macOS (the change is Linux-only).Type of Change
Checklist
AI Usage
See our AI usage policy.
Claude helped read the core dump, write the test and the fix, and run the verification described above.