Skip to content

Commit 5a374d8

Browse files
authored
fix: own outfit-fetch pointers so the detached background fetch cannot read a pooled list after release
Sturdiness hardening on the outfit fetch: the detached background provider fetch and the settle predicate now read an owned URN[] snapshot instead of the pooled missingUrns list (released when ExecuteAsync returns), so a future provider that does not copy pointers before its first await cannot turn this into a pooled-list use-after-free. Discriminating EditMode test added; 9/9 green on the rig.
1 parent 09e24d8 commit 5a374d8

2 files changed

Lines changed: 40 additions & 5 deletions

File tree

Explorer/Assets/DCL/Backpack/AvatarSection/Outfits/Commands/CacheOutfitWearablesCommand.cs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,28 +81,33 @@ public async UniTask ExecuteAsync(IReadOnlyCollection<URN>? wearableUrns, BodySh
8181
// whose DTO failed has no metadata to equip and is left out.
8282
private async UniTask FetchMissingDtosIntoResultAsync(List<URN> missingUrns, BodyShape bodyShape, List<IWearable> result, CancellationToken ct)
8383
{
84+
// The background fetch and the settle-predicate can both run after this scope returns,
85+
// once ExecuteAsync has returned missingUrns to the pool (where it may be cleared or
86+
// reused). They read an owned snapshot so they never touch the released pooled list.
87+
URN[] snapshot = missingUrns.ToArray();
88+
8489
UniTask fullFetch = FullFetchAsync();
8590

91+
// fullFetch is the termination backstop: AllMissingDtosSettled can stay false forever for
92+
// a pointer that never starts loading, but the full fetch always completes and ends the wait.
8693
await UniTask.WhenAny(fullFetch, UniTask.WaitUntil(AllMissingDtosSettled, cancellationToken: ct));
8794

88-
foreach (URN urn in missingUrns)
95+
foreach (URN urn in snapshot)
8996
if (TryGetStored(urn, out IWearable w) && w.DTO != null)
9097
result.Add(w);
9198

9299
return;
93100

94-
// The provider copies the pointers into its intention before its first await, so the
95-
// background continuation never touches the pooled list after this scope ends.
96101
async UniTask FullFetchAsync()
97102
{
98-
try { await wearablesProvider.GetByPointersAsync(missingUrns, bodyShape, ct); }
103+
try { await wearablesProvider.GetByPointersAsync(snapshot, bodyShape, ct); }
99104
catch (OperationCanceledException) { }
100105
catch (Exception e) { ReportHub.LogException(e, ReportCategory.OUTFITS); }
101106
}
102107

103108
bool AllMissingDtosSettled()
104109
{
105-
foreach (URN urn in missingUrns)
110+
foreach (URN urn in snapshot)
106111
{
107112
if (!TryGetStored(urn, out IWearable w))
108113
return false;

Explorer/Assets/DCL/Tests/Editor/CacheOutfitWearablesCommandShould.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,36 @@ public async Task CompleteOnDtoSettleAndApplyRefetchedStorageHitWithoutAwaitingA
161161
assetFetchNeverCompletes.TrySetCanceled();
162162
}
163163

164+
[Test]
165+
public async Task DetachedFetchReceivesOwnedSnapshotNotThePooledList()
166+
{
167+
LogAssert.ignoreFailingMessages = true;
168+
169+
IReadOnlyCollection<URN>? handedToFetch = null;
170+
171+
wearablesProvider.GetByPointersAsync(Arg.Any<IReadOnlyCollection<URN>>(), Arg.Any<BodyShape>(), Arg.Any<CancellationToken>(), Arg.Any<List<IWearable>?>())
172+
.Returns(callInfo =>
173+
{
174+
handedToFetch = callInfo.Arg<IReadOnlyCollection<URN>>();
175+
return UniTask.FromResult<IReadOnlyCollection<IWearable>?>(null);
176+
});
177+
178+
var result = new List<IWearable>();
179+
var urns = new List<URN> { RESOLVED_URN, UNRESOLVED_URN };
180+
181+
await command.ExecuteAsync(urns, BodyShape.MALE, CancellationToken.None, result, useFullUrns: true);
182+
183+
// ExecuteAsync has returned, so its pooled missingUrns list is back in the pool and free to be
184+
// cleared or reused by the next command. The detached fetch it launched must therefore read an
185+
// owned snapshot (a URN[]), never the pooled List<URN>, so pool reuse cannot corrupt it.
186+
Assert.IsNotNull(handedToFetch, "The provider fetch must have been launched for the missing pointer");
187+
Assert.IsInstanceOf<URN[]>(handedToFetch, "The detached fetch must receive an owned snapshot, not the pooled List<URN>");
188+
189+
var handed = new List<URN>(handedToFetch);
190+
Assert.AreEqual(1, handed.Count, "The snapshot must carry exactly the missing pointers");
191+
Assert.AreEqual(UNRESOLVED_URN, handed[0].ToString(), "The snapshot must carry the unresolved pointer to the provider");
192+
}
193+
164194
private static async UniTask PumpAsync()
165195
{
166196
for (var i = 0; i < 120; i++)

0 commit comments

Comments
 (0)