CNA-BUG-150: StorageDevice::EnsureStorageRoot caches the process-wide storage root in unsynchronised statics

CNA snapshot c1c316b9  ·  Known Issues › Current bugs  ·  source links pinned to c1c316b9

The storage root is cached in a plain static bool and std::string with no lock or atomic, so two threads touching storage for the first time both resolve it and write the string concurrently, a data race, and a reader may see the flag before the string.

Identifier

CNA-BUG-150

Category

Bug

Subsystem

Storage

Status

Open

Verified against

CNA c1c316b9 (c1c316b9c7a846ce8002809c151fcd1af14942c9)

Severity

Low (a triage suggestion, not a project priority)

Evidence basis

Source-verified: read at TARGET, not executed

Tests touching this area

Yes: see Current tests

Affected contract

Microsoft::Xna::Framework::Storage::StorageDevice::GetStorageRootEXT() (“Absolute path to the storage root”) and every storage operation that resolves the root

Expected behaviour

GetStorageRootEXT() and the container operations built on it always see the absolute root. SetAppNameEXT is documented to be called once at startup before storage is used, so renaming concurrently is a caller error, but two threads touching storage for the first time is not prohibited anywhere.

Actual behaviour at TARGET

EnsureStorageRoot in StorageDevice.cpp tests storageRootInitialized_, resolves the path, assigns storageRoot_ and sets the flag last. That order was corrected by 8a67da536, which removed the single-threaded empty-root latch. storageRoot_, appName_ and storageRootInitialized_ are still plain statics with no lock or atomic. Two threads that arrive before the first resolution finishes both resolve the root and both assign the std::string, a data race (undefined behaviour). Nothing orders the plain bool store after the string store, so another core may see the flag set while the string is still being written.

Source locations

• modules/storage/src/StorageDevice.cpp — StorageDevice::EnsureStorageRoot (flag set after storageRoot_, no lock), SetAppNameEXT, GetStorageRootEXT
• modules/storage/include/Microsoft/Xna/Framework/Storage/StorageDevice.hpp — static storageRoot_, appName_, storageRootInitialized_; GetStorageRootEXT and SetAppNameEXT documentation

Evidence

Checked by reading StorageDevice.cpp and its header at c1c316b9. The race is a consequence of the statement order; it was not provoked. Not executed.

Re-verified at b0e97bb1: 8a67da536 (SAMPLE-070) moved the flag after the assignment; the three statics in StorageDevice.hpp are still unsynchronised. Not executed.

Focused reproduction

No focused reproduction is known. Nothing has been invented here; the evidence above is what exists.

Current tests

StorageDeviceTests.cpp is single-threaded; no test covers concurrent first access.

Regression test

A ThreadSanitizer test that resets the root with SetAppNameEXT and then calls GetStorageRootEXT() from several threads at once, expecting the same non-empty absolute path from every thread and no reported race.

Blast radius

Games that save or query storage from more than one thread before the root has been resolved once (for example a background save racing the first storage query of the main thread). Single-threaded storage use, and any use after the first successful resolution, is unaffected.

Workaround

Call StorageDevice::GetStorageRootEXT() once on the main thread at startup, after SetAppNameEXT, before any other thread touches storage.