fix: retain private Combo creation rollback ownership

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','test_provider_shutdown_drain.py','test_ffx_release.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','test_ffx_create.py','test_combo_create.py')) {
             $command = 'call "' + $vcvars + '" && python tests\' + $script + ' --compiler cl'
             & $env:ComSpec /d /c $command
             if ($LASTEXITCODE) { throw "Compatibility routing test failed: $script" }

OptiScaler/framegen/nvngx/IFGNvngx.h

@@ -33,6 +33,9 @@ class IFGNvngx
                                             const NVSDK_NGX_FeatureCommonInfo* InFeatureInfo) = 0;
     virtual NVSDK_NGX_Result D3D12_Shutdown() = 0;
     virtual NVSDK_NGX_Result D3D12_Shutdown1(ID3D12Device* InDevice) = 0;
+    // Internal pre-shutdown hook, under exclusive Nvngx_FG transition admission.
+    // Drain owned objects that failed creation before a public token existed.
+    virtual NVSDK_NGX_Result D3D12_DrainPending(ID3D12Device* InDevice) { return NVSDK_NGX_Result_Success; }
     virtual NVSDK_NGX_Result D3D12_GetScratchBufferSize(NVSDK_NGX_Feature InFeatureId,
                                                         const NVSDK_NGX_Parameter* InParameters,
                                                         size_t* OutSizeInBytes) = 0;

OptiScaler/framegen/nvngx/Nvngx_Combo.cpp

@@ -44,11 +44,47 @@ NVSDK_NGX_Result Nvngx_Combo::D3D12_Shutdown() { return D3D12_Shutdown1(nullptr)
 
 NVSDK_NGX_Result Nvngx_Combo::D3D12_Shutdown1(ID3D12Device* InDevice)
 {
+    const auto pending = D3D12_DrainPending(InDevice);
+    if (pending != NVSDK_NGX_Result_Success)
+        return pending;
     const auto resultArturs = ShutdownChild(_artursInitAttempts, *artursProvider, InDevice);
     const auto resultFfx = ShutdownChild(_ffxInitAttempts, *ffxProvider, InDevice);
     return resultArturs != NVSDK_NGX_Result_Success ? resultArturs : resultFfx;
 }
 
+void Nvngx_Combo::ForgetPending(const std::shared_ptr<PendingCreate>& pending)
+{
+    std::lock_guard lock(_pendingMutex);
+    std::erase(_pendingCreates, pending);
+}
+
+NVSDK_NGX_Result Nvngx_Combo::D3D12_DrainPending(ID3D12Device* InDevice)
+{
+    std::vector<std::shared_ptr<PendingCreate>> pending;
+    {
+        std::lock_guard lock(_pendingMutex);
+        for (const auto& entry : _pendingCreates)
+            if (!InDevice || entry->device.Get() == InDevice)
+                pending.push_back(entry);
+    }
+    // An interrupted third-party call or error with non-null output does not
+    // prove ownership. Do not pass that pointer to Release or close its parent.
+    for (const auto& entry : pending)
+        if (entry->uncertain)
+        {
+            LOG_ERROR("Combo creation ownership is uncertain; shutdown retained until process restart");
+            return NVSDK_NGX_Result_FAIL_NotInitialized;
+        }
+    for (const auto& entry : pending)
+    {
+        const auto result = ReleaseChildren(entry->handle.get());
+        if (result != NVSDK_NGX_Result_Success)
+            return result;
+        ForgetPending(entry);
+    }
+    return NVSDK_NGX_Result_Success;
+}
+
 NVSDK_NGX_Result Nvngx_Combo::D3D12_GetScratchBufferSize(NVSDK_NGX_Feature InFeatureId,
                                                          const NVSDK_NGX_Parameter* InParameters,
                                                          size_t* OutSizeInBytes)
@@ -64,42 +100,62 @@ NVSDK_NGX_Result Nvngx_Combo::D3D12_GetScratchBufferSize(NVSDK_NGX_Feature InFea
 NVSDK_NGX_Result Nvngx_Combo::D3D12_CreateFeature(ID3D12GraphicsCommandList* InCmdList, NVSDK_NGX_Feature InFeatureID,
                                                   NVSDK_NGX_Parameter* InParameters, NVSDK_NGX_Handle** OutHandle)
 {
-    Nvngx_Combo_Handle** OutOurHandle = (Nvngx_Combo_Handle**) OutHandle;
-
-    if (!OutOurHandle || !InParameters)
+    if (!OutHandle)
+        return NVSDK_NGX_Result_FAIL_InvalidParameter;
+    *OutHandle = nullptr;
+    if (!InParameters || !InCmdList)
         return NVSDK_NGX_Result_FAIL_InvalidParameter;
 
     if (InFeatureID != NVSDK_NGX_Feature_FrameGeneration)
         return NVSDK_NGX_Result_FAIL_FeatureNotSupported;
 
-    *OutOurHandle = new Nvngx_Combo_Handle(lastIdCreated++, nullptr);
-
-    auto resultArturs =
-        artursProvider->D3D12_CreateFeature(InCmdList, InFeatureID, InParameters, &(*OutOurHandle)->artursHandle);
-    auto resultFfx =
-        ffxProvider->D3D12_CreateFeature(InCmdList, InFeatureID, InParameters, &(*OutOurHandle)->ffxHandle);
+    ComPtr<ID3D12Device> device;
+    if (FAILED(InCmdList->GetDevice(IID_PPV_ARGS(&device))) || !device)
+        return NVSDK_NGX_Result_FAIL_PlatformError;
 
-    if (resultArturs != NVSDK_NGX_Result_Success || resultFfx != NVSDK_NGX_Result_Success)
+    auto pending = std::make_shared<PendingCreate>();
+    pending->handle = std::make_unique<Nvngx_Combo_Handle>();
+    pending->handle->Id = lastIdCreated++;
+    // Retain identity too: a failed child must not let this address be reused
+    // for an unrelated device while its private cleanup is still pending.
+    pending->device.Attach(device.Detach());
     {
-        if (resultArturs == NVSDK_NGX_Result_Success)
-            artursProvider->D3D12_ReleaseFeature((*OutOurHandle)->artursHandle);
-
-        if (resultFfx == NVSDK_NGX_Result_Success)
-            ffxProvider->D3D12_ReleaseFeature((*OutOurHandle)->ffxHandle);
-
-        delete *OutOurHandle;
-        *OutOurHandle = nullptr;
-
+        std::lock_guard lock(_pendingMutex);
+        _pendingCreates.push_back(pending);
+    }
+    // Register before the first external call. Exceptions retain the private
+    // ledger, and no partially constructed handle is exposed to the caller.
+    pending->uncertain = true;
+    const auto resultArturs =
+        artursProvider->D3D12_CreateFeature(InCmdList, InFeatureID, InParameters, &pending->uncertainHandle);
+    if (resultArturs != NVSDK_NGX_Result_Success || !pending->uncertainHandle)
+    {
+        if (resultArturs != NVSDK_NGX_Result_Success && !pending->uncertainHandle)
+            ForgetPending(pending);
+        return NVSDK_NGX_Result_Fail;
+    }
+    pending->handle->artursHandle = pending->uncertainHandle;
+    pending->uncertainHandle = nullptr;
+    pending->uncertain = false;
+
+    // FFX Create has a strong output guarantee: failure/exception publishes no
+    // object. Its successful output and the first child stay ledger-owned here.
+    const auto resultFfx =
+        ffxProvider->D3D12_CreateFeature(InCmdList, InFeatureID, InParameters, &pending->handle->ffxHandle);
+    if (resultFfx != NVSDK_NGX_Result_Success)
+    {
+        if (ReleaseChildren(pending->handle.get()) == NVSDK_NGX_Result_Success)
+            ForgetPending(pending);
         return NVSDK_NGX_Result_Fail;
     }
 
+    ForgetPending(pending);
+    *OutHandle = reinterpret_cast<NVSDK_NGX_Handle*>(pending->handle.release());
     return NVSDK_NGX_Result_Success;
 }
 
-NVSDK_NGX_Result Nvngx_Combo::D3D12_ReleaseFeature(NVSDK_NGX_Handle* InHandle)
+NVSDK_NGX_Result Nvngx_Combo::ReleaseChildren(Nvngx_Combo_Handle* InOurHandle)
 {
-    Nvngx_Combo_Handle* InOurHandle = (Nvngx_Combo_Handle*) InHandle;
-
     if (!InOurHandle)
         return NVSDK_NGX_Result_FAIL_InvalidParameter;
 
@@ -115,6 +171,15 @@ NVSDK_NGX_Result Nvngx_Combo::D3D12_ReleaseFeature(NVSDK_NGX_Handle* InHandle)
     if (InOurHandle->artursHandle || InOurHandle->ffxHandle)
         return NVSDK_NGX_Result_Fail;
 
+    return NVSDK_NGX_Result_Success;
+}
+
+NVSDK_NGX_Result Nvngx_Combo::D3D12_ReleaseFeature(NVSDK_NGX_Handle* InHandle)
+{
+    auto* InOurHandle = reinterpret_cast<Nvngx_Combo_Handle*>(InHandle);
+    const auto result = ReleaseChildren(InOurHandle);
+    if (result != NVSDK_NGX_Result_Success)
+        return result;
     delete InOurHandle;
     return NVSDK_NGX_Result_Success;
 }

OptiScaler/framegen/nvngx/Nvngx_Combo.h

@@ -4,6 +4,9 @@
 #include "Nvngx_Arturs.h"
 #include <d3d12.h>
 #include <unordered_set>
+#include <mutex>
+#include <vector>
+#include <wrl/client.h>
 
 struct Nvngx_Combo_Handle
 {
@@ -22,6 +25,20 @@ class Nvngx_Combo : public IFGNvngx
     std::unique_ptr<Nvngx_Arturs> artursProvider = nullptr;
     std::unique_ptr<Nvngx_FFX> ffxProvider = nullptr;
 
+    struct PendingCreate
+    {
+        std::unique_ptr<Nvngx_Combo_Handle> handle;
+        Microsoft::WRL::ComPtr<ID3D12Device> device;
+        NVSDK_NGX_Handle* uncertainHandle = nullptr;
+        bool uncertain = false;
+    };
+    // Create calls may run concurrently. Hold this mutex only for bookkeeping,
+    // never across child callbacks; Shutdown is excluded by outer admission.
+    std::mutex _pendingMutex;
+    std::vector<std::shared_ptr<PendingCreate>> _pendingCreates;
+    void ForgetPending(const std::shared_ptr<PendingCreate>& pending);
+    NVSDK_NGX_Result ReleaseChildren(Nvngx_Combo_Handle* handle);
+
     // The outer Nvngx_FG transition admission serializes Init/Shutdown.
     // Record each child separately so retry never closes a completed child twice.
     std::unordered_set<ID3D12Device*> _artursInitAttempts;
@@ -69,6 +86,7 @@ class Nvngx_Combo : public IFGNvngx
     NVSDK_NGX_Result D3D12_Shutdown() override;
 
     NVSDK_NGX_Result D3D12_Shutdown1(ID3D12Device* InDevice) override;
+    NVSDK_NGX_Result D3D12_DrainPending(ID3D12Device* InDevice) override;
 
     NVSDK_NGX_Result D3D12_GetScratchBufferSize(NVSDK_NGX_Feature InFeatureId, const NVSDK_NGX_Parameter* InParameters,
                                                 size_t* OutSizeInBytes) override;

OptiScaler/framegen/nvngx/Nvngx_FFX.cpp

@@ -70,27 +70,32 @@ NVSDK_NGX_Result Nvngx_FFX::D3D12_GetScratchBufferSize(NVSDK_NGX_Feature InFeatu
 NVSDK_NGX_Result Nvngx_FFX::D3D12_CreateFeature(ID3D12GraphicsCommandList* InCmdList, NVSDK_NGX_Feature InFeatureID,
                                                 NVSDK_NGX_Parameter* InParameters, NVSDK_NGX_Handle** OutHandle)
 {
-    Nvngx_FFX_Handle** OutOurHandle = (Nvngx_FFX_Handle**) OutHandle;
-
-    if (!Init())
-        return NVSDK_NGX_Result_Fail;
-
-    if (!OutOurHandle || !InParameters)
+    if (!OutHandle)
+        return NVSDK_NGX_Result_FAIL_InvalidParameter;
+    *OutHandle = nullptr;
+    if (!InParameters || !InCmdList)
         return NVSDK_NGX_Result_FAIL_InvalidParameter;
 
     if (InFeatureID != NVSDK_NGX_Feature_FrameGeneration)
         return NVSDK_NGX_Result_FAIL_FeatureNotSupported;
 
-    ID3D12Device* pDevice = nullptr;
+    if (!Init())
+        return NVSDK_NGX_Result_Fail;
 
-    if (InCmdList->GetDevice(IID_PPV_ARGS(&pDevice)) != S_OK)
+    ComPtr<ID3D12Device> device;
+    if (FAILED(InCmdList->GetDevice(IID_PPV_ARGS(&device))) || !device)
         return NVSDK_NGX_Result_FAIL_PlatformError;
 
-    // Can't create FFX context yet, missing data
-    *OutOurHandle = new Nvngx_FFX_Handle(lastIdCreated++, nullptr, pDevice);
+    // Keep creation private until every potentially throwing operation is done.
+    // There is no FFX context yet; the temporary COM owner also covers allocation
+    // or parameter-read exceptions, without publishing a half-created handle.
+    auto handle = std::make_unique<Nvngx_FFX_Handle>();
+    handle->Id = lastIdCreated++;
+    InParameters->Get("Width", &handle->swapchainWidth);
+    InParameters->Get("Height", &handle->swapchainHeight);
 
-    InParameters->Get("Width", &(*OutOurHandle)->swapchainWidth);
-    InParameters->Get("Height", &(*OutOurHandle)->swapchainHeight);
+    handle->device = device.Detach();
+    *OutHandle = reinterpret_cast<NVSDK_NGX_Handle*>(handle.release());
 
     return NVSDK_NGX_Result_Success;
 }

OptiScaler/framegen/nvngx/Nvngx_FG.cpp

@@ -296,15 +296,22 @@ NVSDK_NGX_Result Nvngx_FG::DrainHandles(HandleApi api, const void* device)
             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;
+    auto* provider = _provider.Peek();
+    if (!provider)
+        return keys.empty() ? NVSDK_NGX_Result_Success : NVSDK_NGX_Result_FAIL_NotInitialized;
     // 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;
+
+    // Failed Combo creation can own children without any public token. Drain
+    // those before native shutdown even when the registry is empty.
+    if (api == HandleApi::D3D12)
+    {
+        const auto result = provider->D3D12_DrainPending(static_cast<ID3D12Device*>(const_cast<void*>(device)));
+        if (result != NVSDK_NGX_Result_Success)
+            return result;
+    }
 
     for (const auto* key : keys)
         _handles.SuspendReads(key);

docs/PRIVATE_CREATE_ROLLBACK_20260926.md

@@ -0,0 +1,78 @@
+# Private feature creation rollback — 2026-09-26
+
+## Confirmed source defects
+
+`Nvngx_Combo::D3D12_CreateFeature` published its wrapper before either child
+completed creation. If a child failed, it ignored the successful child's Release
+result and deleted the wrapper. In particular, the newly explicit FFX/Combo
+release failure handling could not recover ownership already lost during Create.
+An exception between child calls also left an unpublished child outside the
+public token registry. Draining only published tokens before parent shutdown was
+therefore incomplete.
+
+`Nvngx_FFX::D3D12_CreateFeature` acquired a raw COM device reference before
+allocating its wrapper, published that wrapper before reading parameters, and
+did not reject a null command list or successful GetDevice with a null output.
+Allocation and parameter-read exceptions could leak ownership or expose partial
+output. These are code-level findings; no claim is made that they explain every
+reported Witcher/ZZZ crash.
+
+## Changes
+
+- FFX keeps its wrapper and device reference private with RAII. It publishes
+  output only after parameter reads finish. Invalid arguments, initialization
+  errors, device-query errors, allocation failures and exceptions leave null
+  output. FFX context creation still occurs later in Evaluate.
+- Combo registers private ownership before calling either child. Successful
+  children remain recorded when rollback fails or throws; the public output
+  stays null. The pending record owns a COM reference so its device address cannot
+  be reused before retry completes. Failed first-child creation no longer starts
+  the second child needlessly.
+- Shared `ReleaseChildren` preserves the existing partial-release retry rules
+  for both published and private wrappers. Successful cleanup is not repeated.
+- Internal `D3D12_DrainPending` runs before native shutdown even when there are no
+  public tokens. Combo's direct Shutdown also checks its private ledger. Failed
+  private cleanup blocks parent teardown and preserves Init obligations.
+- An exception inside the third-party Arturs Create, a failure with non-null
+  output, or success with null output leaves uncertain ownership. No guessed
+  pointer is sent to Release; shutdown of that device is refused and logged until
+  process restart. This intentionally prioritizes retention over false success.
+- Only short metadata operations hold the private ledger mutex. Child calls run
+  outside it. Exclusive outer transition admission excludes Shutdown from
+  concurrent Create; ordinary Create calls can still execute concurrently.
+- Provider drain no longer treats an empty public registry as proof that no
+  private objects remain. If a provider exists during process detach, it refuses
+  to start the drain under the possible loader lock, including an empty registry.
+
+## Validation
+
+Local extracted-source C++ tests compiled with Zig's C++ driver:
+
+| Suite | Checks | Scope |
+| --- | ---: | --- |
+| `test_ffx_create.py` | 15 | Entire production Create/Release and real wrapper types; counted COM, injected allocation and parameter exceptions |
+| `test_combo_create.py` | 56 | Complete Create/rollback/private drain/Shutdown, actual ledger members, SDK stand-ins, device isolation and concurrent Create |
+| `test_provider_shutdown_drain.py` | 50 | Actual outer drain/coordinators/registry; includes four new private-drain cases |
+| `test_combo_release.py` | 44 | Affected published partial-release behavior |
+| `test_combo_shutdown.py` | 132 | Affected child close footprints; private hook stubbed here and real in new suite |
+| `test_ffx_release.py` | 22 | Context destruction and device-reference ordering |
+| `test_shutdown_routing.py` | 56 | Exported shutdown routing |
+| `test_provider_handle_api.py` | 42 | Existing API-specific token routing |
+
+Three in-memory negative controls were rejected by executable assertions:
+transferring the FFX device reference before parameter reads; treating live
+children as successful rollback; restoring the outer empty-registry early
+return. Production files were not mutated by these controls.
+
+The two new suites are included in the Windows build workflow. Full DLL build,
+MSVC test results and artifact identity are recorded below once available.
+
+## Remaining scope
+
+This closes the known owned Combo partial-Create gap; it does not establish
+arbitrary third-party exception recovery. Repeated Init with old features,
+native Init transaction ordering, and concurrent ordinary Evaluate scratch-state
+access remain separate lifecycle work. No installer changes, game files, driver
+profiles, native SL1 policy or frame-multiplier algorithms were changed.
+Actual GPU/game behavior remains unverified at the user's request. No aurora
+synchronization or Release is implied by these focused checks.