Skip to content

Commit 11bf92e

Browse files
committed
fix: reserve provider token storage before native creation
1 parent f2cd07b commit 11bf92e

5 files changed

Lines changed: 223 additions & 3 deletions

File tree

‎.github/workflows/just_build_no_signature.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ jobs:
128128
$vswhere = "${env:ProgramFiles(x86)}\Microsoft Visual Studio\Installer\vswhere.exe"
129129
$vs = & $vswhere -latest -products * -requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64 -property installationPath
130130
$vcvars = Join-Path $vs 'VC\Auxiliary\Build\vcvars64.bat'
131-
foreach ($name in @('Aurora_DLSSG_Frame_Guard_Tests','Aurora_Provider_Handle_Tests','Aurora_Input_Lock_Tests','Aurora_Streamline_Binding_Tests','Aurora_Provider_Create_Tests','Aurora_Provider_Publication_Tests','Aurora_Native_Device_Tests')) {
131+
foreach ($name in @('Aurora_DLSSG_Frame_Guard_Tests','Aurora_Provider_Handle_Tests','Aurora_Input_Lock_Tests','Aurora_Streamline_Binding_Tests','Aurora_Provider_Create_Tests','Aurora_Provider_Publication_Tests','Aurora_Native_Device_Tests','Aurora_Provider_Publication_Allocation_Tests')) {
132132
$command = 'call "' + $vcvars + '" && cl /nologo /EHsc /std:c++20 /I OptiScaler tests\' + $name + '.cpp /Fe:x64\' + $name + '.exe && x64\' + $name + '.exe'
133133
& $env:ComSpec /d /c $command
134134
if ($LASTEXITCODE) { throw "Compatibility guard test failed: $name" }

‎OptiScaler/framegen/ProviderHandleCreation.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@ Result CreateProviderHandle(Registry& registry, PublicHandle** output, Value ini
1212
return invalidParameter;
1313
*output = nullptr;
1414
auto pending = registry.Prepare(std::move(initial));
15+
registry.Reserve(pending);
16+
struct Reservation
17+
{
18+
Registry& registry;
19+
decltype(pending)& entry;
20+
~Reservation() { registry.CancelUnpublished(entry); }
21+
} reservation { registry, pending };
1522
const auto result = std::forward<Create>(create)(pending->value);
1623
if (result != success)
1724
return result;

‎OptiScaler/framegen/ProviderHandleRegistry.h‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,37 @@ template <typename Value> class ProviderHandleRegistry
2020
bool releasing = false;
2121
bool retired = false;
2222
bool readsSuspended = false;
23+
bool published = false; // Protected by the registry mutex.
2324
explicit Entry(Value v) : value(std::move(v)) {}
2425
};
2526
using Pending = std::shared_ptr<Entry>;
2627

2728
static Pending Prepare(Value value) { return std::make_shared<Entry>(std::move(value)); }
2829

30+
// Allocate the map node/buckets before entering a provider Create callback.
31+
// Reserved entries remain invisible to routing, Read, Release and drain.
32+
void Reserve(const Pending& entry)
33+
{
34+
std::scoped_lock lock(_mutex);
35+
_entries.emplace(&entry->value, entry);
36+
}
37+
38+
void CancelUnpublished(const Pending& entry)
39+
{
40+
std::scoped_lock lock(_mutex);
41+
if (!entry->published)
42+
_entries.erase(&entry->value);
43+
}
44+
2945
Value* Publish(const Pending& entry)
3046
{
3147
std::scoped_lock lock(_mutex);
3248
auto* key = &entry->value;
33-
_entries.emplace(key, entry);
49+
// CreateProviderHandle reserves this exact entry before native work;
50+
// that route performs no allocation after native creation succeeds.
51+
if (!_entries.contains(key))
52+
_entries.emplace(key, entry);
53+
entry->published = true;
3454
return key;
3555
}
3656

@@ -42,6 +62,8 @@ template <typename Value> class ProviderHandleRegistry
4262
std::scoped_lock lock(_mutex);
4363
for (const auto& [key, entry] : _entries)
4464
{
65+
if (!entry->published)
66+
continue;
4567
std::scoped_lock entryLock(entry->mutex);
4668
if (!entry->retired && predicate(static_cast<const Value&>(entry->value)))
4769
keys.push_back(key);
@@ -148,7 +170,7 @@ template <typename Value> class ProviderHandleRegistry
148170
{
149171
std::scoped_lock lock(_mutex);
150172
const auto it = _entries.find(key);
151-
return it == _entries.end() ? nullptr : it->second;
173+
return it == _entries.end() || !it->second->published ? nullptr : it->second;
152174
}
153175
std::mutex _mutex;
154176
std::unordered_map<const void*, Pending> _entries;

‎docs/PRIVATE_CREATE_ROLLBACK_20260926.md‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,58 @@ access remain separate lifecycle work. No installer changes, game files, driver
7676
profiles, native SL1 policy or frame-multiplier algorithms were changed.
7777
Actual GPU/game behavior remains unverified at the user's request. No aurora
7878
synchronization or Release is implied by these focused checks.
79+
80+
### Next implementation boundary checked against the current source
81+
82+
- Exported D3D12 `Init_Ext` publishes path/application metadata and calls native
83+
`RunDx12Init` before it reaches `Nvngx_FG::D3D12_Init_Ext` and its transition
84+
admission. A busy provider can therefore reject after earlier side effects.
85+
The analogous Vulkan entry and delegated Init variants need the same review.
86+
- Direct replacement-provider Init functions still admit repeated calls using
87+
the attempted-Init sets as cleanup records, not readiness/idempotence records.
88+
Native `NativeDeviceLifecycle::Initialize` already returns success for a known
89+
ready device. Do not incorrectly replace attempted-Init obligations with a
90+
success-only set or allow a failed partial Init to masquerade as ready.
91+
- Any new outer Init coordinator must acquire admission before metadata/native
92+
side effects and use an explicit internal delegation route. A thread-local
93+
"already owns transition" shortcut would let native callbacks bypass exclusion;
94+
the existing `_skipInit` flags only describe path/native-call delegation and
95+
must not become blanket authorization for reentrant mutation.
96+
- D3D11, D3D12 and Vulkan publish shared metadata through individually safe
97+
snapshots. That does not make the combined path/application/project/logging
98+
updates atomic. Direct `NVNGXProxy::InitDx12/InitVulkan` consumers also read these
99+
snapshots and need consideration when defining the outer transaction.
100+
- Ordinary replacement Evaluate still admits concurrent readers. D3D12 uses
101+
process-wide `_hudCopy`; `Nvngx_DllProxy::D3D12_EvaluateFeature` has a static depth
102+
ring counter, provider-wide depth buffers and uses `currentD3D12Device` for
103+
allocation. Per-token registry lifetime protection alone does not serialize
104+
those mutable resources, nor prove their GPU lifetime or device affinity.
105+
Simply changing every ordinary call to a transition would also reject valid
106+
read/query callbacks; use a deliberately scoped design and explicit tests.
107+
108+
## Adjacent publication-allocation follow-up
109+
110+
After the Combo checkpoint `f2cd07b2`, review found a second ownership gap in
111+
`CreateProviderHandle`: the wrapper was allocated before the native call, but
112+
`ProviderHandleRegistry::Publish` still allocated its hash-map node/buckets after
113+
successful native creation. A bad allocation then lost the native handle despite
114+
successful child creation.
115+
116+
The creation helper now reserves that registry entry before calling the provider.
117+
Reservations are invisible to identity routing, Read, Release and LiveKeys. An
118+
RAII scope removes unpublished reservations on error or exception; successful
119+
publication changes visibility without another map allocation. Existing
120+
Prepare/Publish callers remain supported. No SDK callback runs under the registry
121+
mutex, and immutable public identity is exposed only after creation finishes.
122+
123+
`Aurora_Provider_Publication_Allocation_Tests.cpp` adds 34 executable checks with
124+
allocation failure injected into the actual standard-library allocation path:
125+
entry/node/bucket failures precede native callbacks, and all allocations may fail
126+
after native success without preventing publication. It also checks hidden
127+
reservation visibility, failure/exception cancellation and later Release.
128+
Existing 32 registry, 16 creation, 20 Release admission, 42 API routing, 50 drain
129+
and 44 Combo Release checks passed for the affected code. Removing reservation
130+
in temporary header copies fails the new test at check 3. The new suite is part
131+
of Windows CI. This does not infer ownership from a failing third-party Create
132+
that writes an arbitrary non-null output; its pre-existing conservative contract
133+
is unchanged.
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
#include "../OptiScaler/framegen/ProviderHandleRegistry.h"
2+
#include "../OptiScaler/framegen/ProviderHandleCreation.h"
3+
#include <atomic>
4+
#include <cstdlib>
5+
#include <iostream>
6+
#include <new>
7+
#include <stdexcept>
8+
9+
// Fail individual ordinary allocations, or every allocation after the native
10+
// callback succeeds. No production allocation hooks or test toggles are added.
11+
static thread_local int allocationBudget = -1;
12+
void* operator new(std::size_t size)
13+
{
14+
if (allocationBudget == 0)
15+
throw std::bad_alloc();
16+
if (allocationBudget > 0)
17+
--allocationBudget;
18+
if (auto* p = std::malloc(size ? size : 1))
19+
return p;
20+
throw std::bad_alloc();
21+
}
22+
void operator delete(void* p) noexcept { std::free(p); }
23+
void operator delete(void* p, std::size_t) noexcept { std::free(p); }
24+
25+
struct Handle
26+
{
27+
unsigned id;
28+
int* nativeHandle = nullptr;
29+
};
30+
31+
int main()
32+
{
33+
int checks = 0;
34+
auto check = [&](bool ok)
35+
{
36+
if (!ok)
37+
{
38+
std::cerr << "publication allocation check " << checks + 1 << " failed\n";
39+
std::exit(2);
40+
}
41+
++checks;
42+
};
43+
int native = 9;
44+
bool observedPreflightFailure = false;
45+
bool observedSuccess = false;
46+
// Covers the entry, hash node and possible bucket allocations across STL
47+
// implementations. Every throw must occur before the native callback.
48+
for (int budget = 0; budget != 8; ++budget)
49+
{
50+
ProviderHandleRegistry<Handle> registry;
51+
Handle* out = reinterpret_cast<Handle*>(1);
52+
int calls = 0;
53+
allocationBudget = budget;
54+
bool threw = false;
55+
try
56+
{
57+
CreateProviderHandle(registry, &out, Handle { 1 }, 0, -2, -1,
58+
[&](Handle& h)
59+
{
60+
++calls;
61+
h.nativeHandle = &native;
62+
return 0;
63+
});
64+
}
65+
catch (const std::bad_alloc&)
66+
{
67+
threw = true;
68+
}
69+
allocationBudget = -1;
70+
if (threw)
71+
{
72+
observedPreflightFailure = true;
73+
check(calls == 0 && !out);
74+
check(registry.LiveKeys([](auto&) { return true; }).empty());
75+
}
76+
else
77+
{
78+
observedSuccess = true;
79+
check(calls == 1 && out);
80+
}
81+
}
82+
check(observedPreflightFailure && observedSuccess);
83+
84+
ProviderHandleRegistry<Handle> registry;
85+
Handle* out = nullptr;
86+
const void* privateAddress = nullptr;
87+
auto create = [&](bool fail, bool throws)
88+
{
89+
return CreateProviderHandle(
90+
registry, &out, Handle { 2 }, 0, -2, -1,
91+
[&](Handle& h)
92+
{
93+
privateAddress = &h;
94+
check(!out);
95+
check(!registry.GetIdentity(&h, [](auto& v) { return v.id; }));
96+
check(registry.Read(&h, -3, [](auto&) { return 0; }) == -3);
97+
check(registry.Release(&h, -3, [](auto&) { return 0; }, [](int r) { return r == 0; }) == -3);
98+
check(registry.LiveKeys([](auto&) { return true; }).empty());
99+
if (throws)
100+
throw std::runtime_error("native create");
101+
if (fail)
102+
return -1;
103+
h.nativeHandle = &native;
104+
allocationBudget = 0; // No allocation may follow native success.
105+
return 0;
106+
});
107+
};
108+
check(create(true, false) == -1 && !out);
109+
check(!registry.GetIdentity(privateAddress, [](auto& v) { return v.id; }));
110+
bool threw = false;
111+
try
112+
{
113+
create(false, true);
114+
}
115+
catch (const std::runtime_error&)
116+
{
117+
threw = true;
118+
}
119+
check(threw && !out && registry.LiveKeys([](auto&) { return true; }).empty());
120+
int result = -1;
121+
threw = false;
122+
try
123+
{
124+
result = create(false, false);
125+
}
126+
catch (const std::bad_alloc&)
127+
{
128+
threw = true;
129+
}
130+
allocationBudget = -1;
131+
check(!threw && result == 0 && out);
132+
check(registry.Read(out, -3, [](auto& h) { return *h.nativeHandle; }) == native);
133+
check(registry.LiveKeys([](auto&) { return true; }).size() == 1);
134+
check(registry.Release(out, -3, [](auto&) { return 0; }, [](int r) { return r == 0; }) == 0);
135+
std::cout << "PASS: " << checks << " publication allocation checks\n";
136+
}

0 commit comments

Comments
 (0)