fix(compat): drain device-owned FG features before parent shutdown

Author: abc354402600

.github/workflows/just_build_no_signature.yml

@@ -133,7 +133,7 @@ jobs:
             & $env:ComSpec /d /c $command
             if ($LASTEXITCODE) { throw "Compatibility guard test failed: $name" }
         }
-          foreach ($script in @('test_shutdown_routing.py','test_provider_publication_routes.py','test_provider_dll_boundary.py','test_combo_release.py','test_combo_shutdown.py','test_native_operation_routes.py','test_native_module_publication.py','test_native_init_scopes.py','test_ngx_init_paths.py','test_ngx_init_metadata.py','test_provider_release_admission.py','test_provider_call_admission.py','test_provider_init_footprint.py','test_provider_handle_api.py','test_upstream_resource_guards.py','test_provider_settings.py')) {
+          foreach ($script in @('test_shutdown_routing.py','test_provider_publication_routes.py','test_provider_dll_boundary.py','test_combo_release.py','test_combo_shutdown.py','test_native_operation_routes.py','test_native_module_publication.py','test_native_init_scopes.py','test_ngx_init_paths.py','test_ngx_init_metadata.py','test_provider_release_admission.py','test_provider_call_admission.py','test_provider_init_footprint.py','test_provider_handle_api.py','test_upstream_resource_guards.py','test_provider_settings.py','test_provider_shutdown_drain.py','test_ffx_release.py')) {
             $command = 'call "' + $vcvars + '" && python tests\' + $script + ' --compiler cl'
             & $env:ComSpec /d /c $command
             if ($LASTEXITCODE) { throw "Compatibility routing test failed: $script" }

OptiScaler/framegen/ProviderHandleRegistry.h

@@ -5,6 +5,7 @@
 #include <optional>
 #include <unordered_map>
 #include <utility>
+#include <vector>
 
 // Lookup retains the token. Admission is checked under a short per-handle lock;
 // provider callbacks never run under that lock and busy releases never wait.
@@ -32,6 +33,21 @@ template <typename Value> class ProviderHandleRegistry
         return key;
     }
 
+    // Only immutable identity fields may be inspected by the predicate. The
+    // caller must exclude new publication/operations for a lifecycle snapshot.
+    template <typename Predicate> std::vector<const void*> LiveKeys(Predicate&& predicate)
+    {
+        std::vector<const void*> keys;
+        std::scoped_lock lock(_mutex);
+        for (const auto& [key, entry] : _entries)
+        {
+            std::scoped_lock entryLock(entry->mutex);
+            if (!entry->retired && predicate(static_cast<const Value&>(entry->value)))
+                keys.push_back(key);
+        }
+        return keys;
+    }
+
     // Identity remains available after retirement. Retained tokens prevent their
     // addresses being recycled into unrelated handles while the DLL is loaded.
     template <typename Identity>

OptiScaler/framegen/nvngx/Nvngx_FFX.cpp

@@ -102,8 +102,19 @@ NVSDK_NGX_Result Nvngx_FFX::D3D12_ReleaseFeature(NVSDK_NGX_Handle* InHandle)
     if (!InOurHandle)
         return NVSDK_NGX_Result_FAIL_InvalidParameter;
 
+    InOurHandle->releaseStarted = true;
+    if (InOurHandle->fgContext && InOurHandle->fgContext->ctx)
+    {
+        const auto result = FfxApiProxy::D3D12_DestroyContext(&InOurHandle->fgContext->ctx, nullptr);
+        if (result != FFX_API_RETURN_OK)
+            return NVSDK_NGX_Result_Fail;
+        InOurHandle->fgContext->ctx = nullptr;
+    }
+    // Destroy the context before dropping our device reference. On failure the
+    // outer registry keeps this wrapper/device alive for an explicit retry.
+    InOurHandle->fgContext.reset();
     InOurHandle->device->Release();
-    delete InOurHandle; // destroys FFX context automatically
+    delete InOurHandle;
 
     return NVSDK_NGX_Result_Success;
 }
@@ -241,6 +252,9 @@ NVSDK_NGX_Result Nvngx_FFX::D3D12_EvaluateFeature(ID3D12GraphicsCommandList* InC
     if (!InParameters || !InOurHandle)
         return NVSDK_NGX_Result_FAIL_InvalidParameter;
 
+    if (InOurHandle->releaseStarted)
+        return NVSDK_NGX_Result_FAIL_FeatureNotFound;
+
     if (!Init())
         return NVSDK_NGX_Result_Fail;
 

OptiScaler/framegen/nvngx/Nvngx_FFX.h

@@ -29,6 +29,7 @@ struct Nvngx_FFX_Handle
     uint32_t swapchainWidth = 0;
     uint32_t swapchainHeight = 0;
     uint64_t lastFrameId = 0;
+    bool releaseStarted = false;
 
     // for HDR
     bool hdrRangeSet = false;

OptiScaler/framegen/nvngx/Nvngx_FG.cpp

@@ -283,14 +283,53 @@ NVSDK_NGX_Result Nvngx_FG::D3D12_Init_Ext(unsigned long long InApplicationId, co
     return provider->D3D12_Init_Ext(InApplicationId, InApplicationDataPath, InDevice, InSDKVersion, InFeatureInfo);
 }
 
+NVSDK_NGX_Result Nvngx_FG::DrainHandles(HandleApi api, const void* device)
+{
+    // The caller holds exclusive transition admission. Snapshot before any
+    // teardown, and never infer a legacy Vulkan command buffer's device from
+    // mutable process-wide state.
+    const auto keys =
+        _handles.LiveKeys([&](const Nvngx_FG_Handle& handle)
+                          { return handle.api == api && (!device || !handle.device || handle.device == device); });
+    if (device)
+        for (const auto* key : keys)
+            if (_handles.GetIdentity(key, [](const Nvngx_FG_Handle& handle) { return handle.device; }) == nullptr)
+                return NVSDK_NGX_Result_FAIL_NotInitialized;
+
+    if (keys.empty())
+        return NVSDK_NGX_Result_Success;
+    // DLL_PROCESS_DETACH may hold the loader lock. Do not start GPU/provider
+    // destruction there; retain ownership rather than claiming a clean drain.
+    if (State::Instance().isShuttingDown)
+        return NVSDK_NGX_Result_FAIL_NotInitialized;
+    auto* provider = _provider.Peek();
+    if (!provider)
+        return NVSDK_NGX_Result_FAIL_NotInitialized;
+
+    for (const auto* key : keys)
+    {
+        const auto result = _handles.Release(
+            key, NVSDK_NGX_Result_FAIL_FeatureNotFound, NVSDK_NGX_Result_FAIL_NotInitialized,
+            [&](const Nvngx_FG_Handle& handle)
+            {
+                return api == HandleApi::D3D12 ? provider->D3D12_ReleaseFeature(handle.nativeHandle)
+                                               : provider->VULKAN_ReleaseFeature(handle.nativeHandle);
+            },
+            [](NVSDK_NGX_Result result) { return result == NVSDK_NGX_Result_Success; });
+        if (result != NVSDK_NGX_Result_Success)
+            return result;
+    }
+    return NVSDK_NGX_Result_Success;
+}
+
 NVSDK_NGX_Result Nvngx_FG::D3D12_Shutdown()
 {
     return WithDx12Shutdown([&](auto closeProvider) { return closeProvider(nullptr); });
 }
 
 NVSDK_NGX_Result Nvngx_FG::D3D12_Shutdown1(ID3D12Device* InDevice)
 {
-    return WithDx12Shutdown([&](auto closeProvider) { return closeProvider(InDevice); });
+    return WithDx12Shutdown([&](auto closeProvider) { return closeProvider(InDevice); }, InDevice);
 }
 
 NVSDK_NGX_Result Nvngx_FG::D3D12_GetScratchBufferSize(NVSDK_NGX_Feature InFeatureId,
@@ -319,8 +358,13 @@ NVSDK_NGX_Result Nvngx_FG::D3D12_CreateFeature(ID3D12GraphicsCommandList* InCmdL
     if (!lease)
         return NVSDK_NGX_Result_FAIL_NotInitialized;
 
+    Microsoft::WRL::ComPtr<ID3D12Device> device;
+    if (!InCmdList || FAILED(InCmdList->GetDevice(IID_PPV_ARGS(&device))) || !device)
+        return NVSDK_NGX_Result_FAIL_InvalidParameter;
+
     return CreateProviderHandle(
-        _handles, OutHandle, Nvngx_FG_Handle { lastIdCreated++ + NVNGX_PROVIDER_ID_OFFSET, nullptr, HandleApi::D3D12 },
+        _handles, OutHandle,
+        Nvngx_FG_Handle { lastIdCreated++ + NVNGX_PROVIDER_ID_OFFSET, nullptr, HandleApi::D3D12, device.Get() },
         NVSDK_NGX_Result_Success, NVSDK_NGX_Result_FAIL_InvalidParameter, NVSDK_NGX_Result_Fail,
         [&](Nvngx_FG_Handle& handle)
         {
@@ -548,7 +592,7 @@ NVSDK_NGX_Result Nvngx_FG::VULKAN_Shutdown()
 
 NVSDK_NGX_Result Nvngx_FG::VULKAN_Shutdown1(VkDevice InDevice)
 {
-    return WithVulkanShutdown([&](auto closeProvider) { return closeProvider(InDevice); });
+    return WithVulkanShutdown([&](auto closeProvider) { return closeProvider(InDevice); }, InDevice);
 }
 
 NVSDK_NGX_Result Nvngx_FG::VULKAN_GetScratchBufferSize(NVSDK_NGX_Feature InFeatureId,
@@ -601,8 +645,12 @@ NVSDK_NGX_Result Nvngx_FG::VULKAN_CreateFeature1(VkDevice InDevice, VkCommandBuf
     if (!lease)
         return NVSDK_NGX_Result_FAIL_NotInitialized;
 
+    if (!InDevice)
+        return NVSDK_NGX_Result_FAIL_InvalidParameter;
+
     return CreateProviderHandle(
-        _handles, OutHandle, Nvngx_FG_Handle { lastIdCreated++ + NVNGX_PROVIDER_ID_OFFSET, nullptr, HandleApi::Vulkan },
+        _handles, OutHandle,
+        Nvngx_FG_Handle { lastIdCreated++ + NVNGX_PROVIDER_ID_OFFSET, nullptr, HandleApi::Vulkan, InDevice },
         NVSDK_NGX_Result_Success, NVSDK_NGX_Result_FAIL_InvalidParameter, NVSDK_NGX_Result_Fail,
         [&](Nvngx_FG_Handle& handle)
         {

OptiScaler/framegen/nvngx/Nvngx_FG.h

@@ -27,6 +27,7 @@ class Nvngx_FG
         unsigned int id;
         NVSDK_NGX_Handle* nativeHandle = nullptr;
         HandleApi api = HandleApi::D3D12;
+        const void* device = nullptr; // Legacy Vulkan Create has no provable device identity.
     };
 
     static inline std::atomic_uint32_t lastIdCreated = 0;
@@ -42,15 +43,20 @@ class Nvngx_FG
     static std::unique_ptr<IFGNvngx> createProvider();
     static ProviderLookup<IFGNvngx> lookupProvider();
     static IFGNvngx* getProvider();
+    static NVSDK_NGX_Result DrainHandles(HandleApi api, const void* device);
 
   public:
     // The exported API holds this admission through native shutdown, provider
     // shutdown and local cleanup. A rejected transition changes none of them.
-    template <typename Callback> static NVSDK_NGX_Result WithDx12Shutdown(Callback&& callback)
+    template <typename Callback>
+    static NVSDK_NGX_Result WithDx12Shutdown(Callback&& callback, ID3D12Device* device = nullptr)
     {
         auto lease = _calls.TryTransition();
         if (!lease)
             return NVSDK_NGX_Result_FAIL_NotInitialized;
+        const auto drained = DrainHandles(HandleApi::D3D12, device);
+        if (drained != NVSDK_NGX_Result_Success)
+            return drained;
         return std::forward<Callback>(callback)(
             [](ID3D12Device* device)
             {
@@ -71,11 +77,15 @@ class Nvngx_FG
             });
     }
 
-    template <typename Callback> static NVSDK_NGX_Result WithVulkanShutdown(Callback&& callback)
+    template <typename Callback>
+    static NVSDK_NGX_Result WithVulkanShutdown(Callback&& callback, VkDevice device = nullptr)
     {
         auto lease = _calls.TryTransition();
         if (!lease)
             return NVSDK_NGX_Result_FAIL_NotInitialized;
+        const auto drained = DrainHandles(HandleApi::Vulkan, device);
+        if (drained != NVSDK_NGX_Result_Success)
+            return drained;
         return std::forward<Callback>(callback)(
             [](VkDevice device)
             {

OptiScaler/inputs/NVNGX_DLSS_Dx12.cpp

@@ -412,7 +412,8 @@ static NVSDK_NGX_Result ShutdownD3D12(ID3D12Device* device)
             State::Instance().nvngxDx12Inited = false;
             shutdown = false;
             return NVSDK_NGX_Result_Success;
-        });
+        },
+        device);
 }
 
 NVSDK_NGX_API NVSDK_NGX_Result NVSDK_NGX_D3D12_Shutdown(void) { return ShutdownD3D12(nullptr); }

OptiScaler/inputs/NVNGX_DLSS_Vk.cpp

@@ -1202,7 +1202,8 @@ static NVSDK_NGX_Result ShutdownVulkan(VkDevice device)
             State::Instance().nvngxVkInited = false;
             shutdown = false;
             return NVSDK_NGX_Result_Success;
-        });
+        },
+        device);
 }
 
 NVSDK_NGX_API NVSDK_NGX_Result NVSDK_NGX_VULKAN_Shutdown(void) { return ShutdownVulkan(nullptr); }

docs/FEATURE_DRAIN_20260926.md

@@ -0,0 +1,38 @@
+# Provider feature drain before parent shutdown — 2026-09-26
+
+## Defects and policy
+
+Parent Shutdown previously ignored the registry of outstanding replacement FG handles. DLL providers could destroy their contexts while a public token remained readable; FFX's parent Shutdown is a no-op, so merely retiring that token would leak its feature context and owned D3D12 device reference. Combo can retain one child after a failed Release, requiring retry before either parent is closed.
+
+Normal shutdown now acquires the existing exclusive provider transition admission, selects live features for the requested API/device, calls their actual provider ReleaseFeature, and retires each public token only on success. Only after the drain succeeds does it enter the existing native/provider shutdown callback and local cleanup. No registry lock is held while calling provider/native code. Successful releases remain retired if a later release or parent shutdown fails; retry skips them. Failed releases and exceptions retain ownership and do not proceed to parent close. This is a partial-progress transaction, not a rollback that recreates released features.
+
+Identity tokens remain retained until DLL unload, so successful Shutdown followed by Init cannot resurrect a previously drained token through address reuse. This closes that specific stale-handle path; repeated Init without a preceding successful close remains separate work.
+
+## Device identity and conservative boundaries
+
+- D3D12 Create obtains its device from the actual command list, with a temporary COM owner spanning creation/publication. Null command lists, failed GetDevice and null returned device are rejected with an empty output. Stored identity is immutable and non-owning; it is only compared, never dereferenced for cleanup.
+- Vulkan CreateFeature1 records its explicit device and rejects null. Legacy Vulkan CreateFeature has no device argument and remains explicitly unknown; mutable global vkDevice is not used to guess.
+- Device-specific shutdown with any matching-API unknown-device live token returns NotInitialized before releasing anything. The caller can release that token explicitly or request global shutdown. Global shutdown can drain unknown-device tokens of its API safely through their provider; it does not drain the other API.
+- Once DLL_PROCESS_DETACH has set isShuttingDown, live-feature drain is rejected before entering GPU/provider destruction under a possible loader lock. No successful cleanup is claimed. This does not redesign the DLL's other existing detach behavior.
+- ReleaseFeature's existing provider/driver contract still applies: CPU exclusion is not a GPU fence or proof that submitted work has finished. No real GPU tests were performed, per user instruction.
+
+## FFX feature cleanup repair
+
+FFX Release previously dropped its device reference before deleting the wrapper whose destructor destroys the context. It also discarded context destruction failures in the destructor while reporting successful Release. Release now explicitly destroys the context while the owned device reference is alive. Error/exception retains the wrapper/reference for retry; Evaluate rejects a wrapper whose release has started. Successful destruction clears the context before resetting the holder, preventing double destruction, then releases the device and deletes the wrapper. If the SDK clears the context even while returning failure, retry does not call DestroyContext on null.
+
+This covers the normal explicit Release path and the shutdown drain that uses it. It does not assert that every separate context-holder destructor elsewhere can recover from a destruction failure.
+
+## Tests and source scope
+
+- 39 new `test_provider_shutdown_drain.py` checks compile the actual Create/Release/drain/coordinator methods with production registry/admission and fake SDK/COM calls. They cover invalid command lists/devices, balanced temporary COM reference, device/API isolation, active-call exclusion, detach rejection, worker-thread reentry, failures/exceptions, partial-progress retry, parent failure, stale tokens after reinitialization and unknown legacy Vulkan identity. D3D12 HUD work after admission remains a stand-in.
+- 22 new `test_ffx_release.py` checks compile the actual FFX holder/destructor, complete Release and Evaluate rejection prefix. Counted SDK/COM stand-ins check destruction order, absent contexts, error/exception retry and context cleared on error. No FFX GPU execution is simulated.
+- Actual exported shutdown tests expand from 52 to 56 checks: failed drain reaches neither native close nor local cleanup and receives the exact requested device. Existing fixtures for unrelated routes model an empty successful drain; the new suite tests the actual drain.
+- Affected 85 admission, 104 Init-footprint, 36 publication-routing, 42 handle-API, 20 Release-admission, 44 Combo Release and 132 Combo shutdown checks passed locally.
+- Three negative controls in temporary copied sources detect removal of device filtering, ignored drain failure, and premature device release. Production files are never mutated by those controls.
+- The source extraction helper now tolerates whitespace in declarations because clang-format wraps the longer coordinator signatures. It still fails if the production declaration is absent.
+
+## Remaining work and synchronization gate
+
+Repeated Init while old features remain, native Init/global configuration transaction ordering, partial Create rollback, and concurrent ordinary Evaluate access to shared HUD/depth resources are not solved by this batch. A legacy Vulkan token without explicit device ownership intentionally constrains device-specific close. Preserve these boundaries in the next continuation; do not invent a device association or label CPU tests as game compatibility proof.
+
+The user on 2026-09-26 authorizes direct synchronization to aurora when the code-validation gate is actually met, without a separate confirmation or synchronization notification. Real-game tests remain deferred. No Release is authorized. This checkpoint alone does not close the remaining boundaries above. Full Windows validation will be appended after Actions completes.