Skip to content

fix(windows): stop DXGI enumeration on any failure #5893

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

Open
abbradar wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(windows): stop DXGI enumeration on any failure #5893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f557b34
fix(windows): stop DXGI enumeration on any failure
abbradar Oct 10, 2026
fd55804
Address SonarCloud issues
abbradar Oct 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions src/platform/windows/display_base.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,8 @@ namespace platf::dxgi {

adapter_t::pointer adapter_p;
for (int tries = 0; tries < 2; ++tries) {
for (int x = 0; factory->EnumAdapters1(x, &adapter_p) != DXGI_ERROR_NOT_FOUND; ++x) {
int x = 0;
while (SUCCEEDED(status = factory->EnumAdapters1(x++, &adapter_p))) {

@ReenigneArcher ReenigneArcher Nov 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Add regression coverage for the failed-enumeration paths

The PR changes six production enumeration loops but adds no automated tests. The manual SSH check does not exercise a failure after a successful result, which is the partial-list defect above. Regression tests are required for changed source behavior.

Add tests/unit/platform/windows/test_dxgi_enumeration.cpp, using ../../../tests_common.h in the test include group and the existing GoogleTest framework. Introduce an injectable factory/adapter seam or a shared production helper so these tests exercise the actual enumeration code without requiring SSH or a particular GPU. There is currently no enumeration injection seam, so a test that merely copies the loops would not protect the fix.

Concrete cases for DxgiEnumerationTest:

  • Make the first adapter call fail with E_FAIL, and the first output call fail with DXGI_ERROR_NOT_CURRENTLY_AVAILABLE; assert no descriptor/pointer access occurs after failure and the call terminates.
  • Return a capturable PRIMARY first, then fail adapter enumeration, output enumeration, or output enumeration on a later adapter; assert the result is empty.
  • Return PRIMARY and SECONDARY followed by DXGI_ERROR_NOT_FOUND; assert both names remain and normal exhaustion produces no error log.
  • Exercise the initialization and diagnostic-tool paths through the same seam/helper, checking that unexpected failures stop enumeration and report the appropriate error.

Document the failure contract in Doxygen alongside the changed methods/helper. A focused test command after building the test target is:

C:\msys64\msys2_shell.cmd -defterm -here -no-start -ucrt64 -c './cmake-build-release/tests/test_sunshine.exe --gtest_filter=DxgiEnumerationTest.*'

These are proposed repository tests; they have not been added or run. The separate review harness covered the seven display-list scenarios described above.

Review completed by Codex | Model: gpt-6.1-sol | Thinking: xhigh

dxgi::adapter_t adapter_tmp {adapter_p};

DXGI_ADAPTER_DESC1 adapter_desc;
Expand All @@ -514,7 +515,8 @@ namespace platf::dxgi {
}

dxgi::output_t::pointer output_p;
for (int y = 0; adapter_tmp->EnumOutputs(y, &output_p) != DXGI_ERROR_NOT_FOUND; ++y) {
int y = 0;
while (SUCCEEDED(status = adapter_tmp->EnumOutputs(y++, &output_p))) {
dxgi::output_t output_tmp {output_p};

DXGI_OUTPUT_DESC desc;
Expand Down Expand Up @@ -549,12 +551,18 @@ namespace platf::dxgi {
break;
}
}
if (FAILED(status) && status != DXGI_ERROR_NOT_FOUND) {
BOOST_LOG(error) << "Failed to enumerate DXGI outputs [0x"sv << util::hex(status).to_string_view() << ']';
}

if (output) {
adapter = std::move(adapter_tmp);
break;
}
}
if (FAILED(status) && status != DXGI_ERROR_NOT_FOUND) {
BOOST_LOG(error) << "Failed to enumerate DXGI adapters [0x"sv << util::hex(status).to_string_view() << ']';
}

if (output) {
break;
Expand Down Expand Up @@ -1085,7 +1093,8 @@ namespace platf {
}

dxgi::adapter_t::pointer adapter_p;
for (int x = 0; factory->EnumAdapters1(x, &adapter_p) != DXGI_ERROR_NOT_FOUND; ++x) {
int x = 0;
while (SUCCEEDED(status = factory->EnumAdapters1(x++, &adapter_p))) {
dxgi::adapter_t adapter {adapter_p};
DXGI_ADAPTER_DESC1 adapter_desc;
adapter->GetDesc1(&adapter_desc);
Expand All @@ -1103,7 +1112,8 @@ namespace platf {
<< " ====== OUTPUT ======"sv << std::endl;

dxgi::output_t::pointer output_p {};
for (int y = 0; adapter->EnumOutputs(y, &output_p) != DXGI_ERROR_NOT_FOUND; ++y) {
int y = 0;
while (SUCCEEDED(status = adapter->EnumOutputs(y++, &output_p))) {
dxgi::output_t output {output_p};

DXGI_OUTPUT_DESC desc;
Expand All @@ -1125,6 +1135,12 @@ namespace platf {
display_names.emplace_back(std::move(device_name));
}
}
if (FAILED(status) && status != DXGI_ERROR_NOT_FOUND) {
BOOST_LOG(error) << "Failed to enumerate DXGI outputs [0x"sv << util::hex(status).to_string_view() << ']';
}
}
if (FAILED(status) && status != DXGI_ERROR_NOT_FOUND) {
BOOST_LOG(error) << "Failed to enumerate DXGI adapters [0x"sv << util::hex(status).to_string_view() << ']';
}
Comment on lines +1138 to 1144

@ReenigneArcher ReenigneArcher Nov 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Discard the display list when enumeration fails

If one display has already been appended and a later EnumOutputs() or EnumAdapters1() returns an unexpected failure, these handlers only log it and display_names() returns the earlier displays as a successful, nonempty result. For example, PRIMARY succeeds and enumeration fails before the active SECONDARY display. video::refresh_displays() then replaces its cached list with [PRIMARY], cannot find SECONDARY, and resets the capture selection to PRIMARY. Returning an empty list instead lets that caller retain its previous list and active display. This also preserves the all-or-nothing requirement documented at the start of this function.

Return immediately from both unexpected-error handlers:

Suggested change
if (FAILED(status) && status != DXGI_ERROR_NOT_FOUND) {
BOOST_LOG(error) << "Failed to enumerate DXGI outputs [0x"sv << util::hex(status).to_string_view() << ']';
}
}
if (FAILED(status) && status != DXGI_ERROR_NOT_FOUND) {
BOOST_LOG(error) << "Failed to enumerate DXGI adapters [0x"sv << util::hex(status).to_string_view() << ']';
}
if (FAILED(status) && status != DXGI_ERROR_NOT_FOUND) {
BOOST_LOG(error) << "Failed to enumerate DXGI outputs [0x"sv << util::hex(status).to_string_view() << ']';
return {};
}
}
if (FAILED(status) && status != DXGI_ERROR_NOT_FOUND) {
BOOST_LOG(error) << "Failed to enumerate DXGI adapters [0x"sv << util::hex(status).to_string_view() << ']';
return {};
}

I reproduced this using the exact function from this head with fake DXGI objects: failure after an output, failure after an adapter, and output failure on a later adapter all returned [PRIMARY]. With these two returns, all seven harness scenarios passed. This was an extracted-function harness, not a hardware or full Sunshine test run.

Review completed by Codex | Model: gpt-6.1-sol | Thinking: xhigh


return display_names;
Expand Down
14 changes: 11 additions & 3 deletions tools/dxgi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,16 @@ int main(int argc, char *argv[]) {
SetProcessDpiAwarenessContext(DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2);

dxgi::factory1_t::pointer factory_p {};
const HRESULT status = CreateDXGIFactory1(IID_IDXGIFactory1, static_cast<void **>(static_cast<void *>(&factory_p)));
HRESULT status = CreateDXGIFactory1(IID_IDXGIFactory1, static_cast<void **>(static_cast<void *>(&factory_p)));
dxgi::factory1_t factory {factory_p};
if (FAILED(status)) {
std::cout << "Failed to create DXGIFactory1 [0x"sv << util::hex(status).to_string_view() << ']' << std::endl;
return -1;
}

dxgi::adapter_t::pointer adapter_p {};
for (int x = 0; factory->EnumAdapters1(x, &adapter_p) != DXGI_ERROR_NOT_FOUND; ++x) {
int x = 0;
while (SUCCEEDED(status = factory->EnumAdapters1(x++, &adapter_p))) {
dxgi::adapter_t adapter {adapter_p};

DXGI_ADAPTER_DESC1 adapter_desc;
Expand All @@ -53,7 +54,8 @@ int main(int argc, char *argv[]) {

dxgi::output_t::pointer output_p {};
bool has_outputs = false;
for (int y = 0; adapter->EnumOutputs(y, &output_p) != DXGI_ERROR_NOT_FOUND; ++y) {
int y = 0;
while (SUCCEEDED(status = adapter->EnumOutputs(y++, &output_p))) {
// Print the header only when we find the first output
if (!has_outputs) {
std::cout << std::endl
Expand All @@ -73,8 +75,14 @@ int main(int argc, char *argv[]) {
std::cout << " AttachedToDesktop : " << (desc.AttachedToDesktop ? "yes" : "no") << std::endl;
std::cout << " Resolution : " << std::format("{}x{}", width, height) << std::endl;
}
if (FAILED(status) && status != DXGI_ERROR_NOT_FOUND) {
std::cout << "Failed to enumerate DXGI outputs [0x"sv << util::hex(status).to_string_view() << ']' << std::endl;
}
std::cout << std::endl;
}
if (FAILED(status) && status != DXGI_ERROR_NOT_FOUND) {
std::cout << "Failed to enumerate DXGI adapters [0x"sv << util::hex(status).to_string_view() << ']' << std::endl;
}

return 0;
}