Repository navigation
Windows SMB: shader disk-cache retained-reader replacement fails; local NTFS passes #10833
Description
Resolved on main via #11434, merge a453b29ebe4b. The selected contract is now in main: atomic production replacement semantics remain; affected retained-reader/mapped-replacement test subcases are omitted only when the filesystem POSIX-rename probe reports error87, and the SMB limitation is documented. This does not add atomic SMB replacement support. Our original real-NAS reproduction remains historical; we have not independently retested the merged change on that NAS. AI-assisted report maintenance: yes (Codex).
Maintainer decision and existing candidate — 26 October 2026
MarcZX10 selected option 1: retain atomic production replacement semantics, document the SMB limitation, and omit only retained-reader replacement assertions when the POSIX rename probe specifically returns ERROR_INVALID_PARAMETER (87). Other replacement failures must remain failures. The rename-aside approach is not accepted because it introduces a missing-path window.
Existing candidate: #11434 by @LeandroLP23, reviewed here at head b95b22cf162225e99d1cb88735e946c32757adc6. Its source extracts the existing Windows POSIX rename operation, checks error 87 in a synthetic filesystem probe, records the limitation, and conditionally omits affected mapped/retained-reader replacement subcases. This is partial subcase coverage, not a claim that SMB supports atomic replacement or that every assertion ran. The patch also corrects extended UNC test paths. We are tracking that contribution rather than implementing a duplicate.
The author's reported NTFS/loopback-SMB runs and GPU cache checks are in #11434; they are independent evidence, not runs performed by us. Our real-NAS reproduction below remains source-pinned historical evidence; we have not independently qualified this candidate on that NAS. Keep this issue open until the accepted change is merged and its remaining scope is clear. The unrelated long-path report #7452 has been closed after its own focused verification.
AI-assisted report maintenance: yes (Codex).
Previous evidence and source-pinned history
What happens
agc_shader_disk_cache fails with cannot replace cache entry with a retained reader when its temporary directory is on the tested Windows-mapped NAS share. The same executable passes when only TEMP/TMP are changed to local NTFS storage.
Title, test or build step
CPU-only CTest agc_shader_disk_cache, specifically verifyAtomicReplacement. No game dump or GPU is needed.
AnyPS5 commit
Executed integration: 29a63ec975692203def21882bfd3f279639bea16, based on main bd8e13428a10d9332c2911816ab672ea1b41418f, with our retained Sonic integration changes. This is not an execution of unchanged main.
The complete production ShaderCacheDirectory.hpp is byte-identical to checked main 390e44c7893c386cec76834152d54570c15df318 (Git blob 9b3893ce2a3d1c403550368f3ce132dfac0550c2). The verifyAtomicReplacement function also matches the integration's base main. A separate minimal program including the actual production header reproduces the difference with synthetic three-/four-byte entries; no AnyPS5 replacement runtime libraries are linked into that probe.
System
Windows 11 Enterprise, build 26200; WinLibs MinGW-w64 GCC 15.2.0/UCRT r7. One local NTFS directory and one writable NAS share mapped as P:. Exact server/filesystem feature support and the failing Windows API/error code have not been determined. GPU/driver: not exercised by this CPU-only check.
Steps to reproduce
Build the real agc_shader_disk_cache_tests target with testing enabled. Use existing writable short temporary directories on the two storage locations, and run from the same shell against the same build:
$env:TEMP = 'P:\tmp'
$env:TMP = $env:TEMP
ctest --test-dir <build> -R '^agc_shader_disk_cache$' -V --output-on-failure
$env:TEMP = 'C:\aps5-test-temp'
$env:TMP = $env:TEMP
ctest --test-dir <build> -R '^agc_shader_disk_cache$' -V --output-on-failureThe executable and build remain unchanged between runs. The failing case completes in 0.56 seconds, within the 30-second CTest timeout. This is the ordinary cache test, not its optional large-file case.
Log
Start 678: agc_shader_disk_cache
678: cannot replace cache entry with a retained reader
1/1 Test #678: agc_shader_disk_cache ............***Failed 0.56 sec
0% tests passed, 1 tests failed out of 1
What was checked
| Check | Local NTFS | Tested mapped NAS share |
|---|---|---|
| Existing CTest, identical executable | Pass | Fail at retained-reader replacement |
| Direct production-header probe, fresh entries | Replacement succeeds 3/3 | Replacement returns false 3/3 |
| Retained reader still sees original bytes | 3/3 | 3/3 |
| Published path has coherent contents | Replacement bytes 3/3 | Original bytes 3/3 |
The probe opens the retained reader with GENERIC_READ and FILE_SHARE_READ | FILE_SHARE_DELETE, matching the fixture, then calls WriteFileAtomically to replace the existing entry. It preserves the original reader and independently reads the published path. No corruption was observed.
CTest executable SHA-256 before/after the storage control: e637cddb5cabb3497e7e635c7b0fbed660698dbc6947de1f497fb940d59e5467.
Related merged #3660 introduced the retained-reader replacement behavior and explicitly did not test the unsupported-filesystem fallback. #5552 concerns delete sharing. Open #7452 concerns long paths; this report uses short paths and isolates replacement while a reader remains open. Searches for the exact error, WriteFileAtomically, FileRenameInfoEx, retained readers, NTFS/SMB and cache replacement found no matching open report.
This establishes a compatibility/test limitation on the tested share, not a general claim that every SMB server fails, a regression introduction, a Sonic startup blocker, or an FPS effect. A local NTFS temporary/cache directory passes this check. It remains to establish the underlying API failure and whether network-filesystem cache replacement should be supported or explicitly documented as unsupported; no non-atomic replacement or relaxed assertion is proposed.
AI-assisted: yes (Codex).
Activity
MarcZX10 commentedon Oct 25, 2026
The NTFS/NAS control and retained-reader probe isolate the condition well. To identify which compatibility path fails, could you capture the exact error from both attempts in the minimal probe: GetLastError immediately after SetFileInformationByHandle with rename class 22, and the std::error_code value/message if the std::filesystem::rename fallback is reached? Also include the NAS protocol/server implementation if available. Those details would distinguish unsupported POSIX-style replacement from a general rename/share failure; I would avoid changing the fallback semantics until that failure mode is known. I have not reproduced this on Windows or that share.
LeandroLP23 commentedon Oct 26, 2026
This reproduces without a NAS, through Windows' own SMB server on loopback, and the failing call is identified.
Reproduction. Windows 11 Home 10.0.26300, WinLibs GCC 15.2, Release,
main204c6747d0(the build also had #11293's one-line test change, unrelated). Sameagc_shader_disk_cache_testsexecutable, onlyTEMP/TMPchanged:TEMPagc_shader_disk_cacheC:\aps5-smbtmp(local NTFS)\\localhost\C$\aps5-smbtmp(the same NTFS volume, through SMB)cannot replace cache entry with a retained readerSo it isn't specific to that NAS or its file system: the SMB path alone is enough.
Error codes. A standalone probe outside the tree replays
WriteFileAtomically's steps: writeentry.bin, open it withGENERIC_READandFILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETElike the test's retained reader, writeentry.bin.tmp, then replace. Each row is a separate attempt from the same starting state:SetFileInformationByHandleclass 22 (FileRenameInfoEx),REPLACE_IF_EXISTS | POSIX_SEMANTICSERROR_INVALID_PARAMETERSetFileInformationByHandleFileRenameInfo,ReplaceIfExistsERROR_ACCESS_DENIEDMoveFileExW(MOVEFILE_REPLACE_EXISTING)std::filesystem::rename(the fallback)error_code13 "Permission denied"Over SMB the redirector rejects the POSIX rename with
ERROR_INVALID_PARAMETER.GetVolumeInformationWreports NTFS and the same flags on both paths, so the volume flags can't predict it. That code is one of the three that reach thestd::filesystem::renamefallback. The fallback is a plain replacing rename, which Windows refuses while any handle is open on the destination, even withFILE_SHARE_DELETE; the local NTFS column shows the same 5/13 once POSIX semantics are dropped. So on such a share an entry can be replaced only while nobody has it open. In the product,WriteFileAtomicallythen returns false: the shader cache doesn't store the new entry, andPipelineCache::savelogscannot writeand keeps the old file. Nothing crashes, but a cache directory on a network share stops updating entries that another process holds open.CacheFileReadTests.cppline 56 ("cannot replace cache path with retained reader") makes the same replacement, so it should fail on such a share too; I haven't run it there.A replacement that works there. In the same probe, renaming the old entry aside (
FileRenameInfo, no replace), renaming the temporary into place, then deleting the aside name succeeds over SMB with the reader open. The destination reads the new bytes, the open reader keeps reading the old ones, and no file is left once the reader closes. That is the behaviourverifyAtomicReplacementchecks, except for atomicity: between the two renames the path doesn't exist, so a concurrent reader sees a miss there, and a crash in that window loses the entry. Two writers racing on one entry would also need care.Two ways forward, both Windows-only:
ERROR_INVALID_PARAMETERfrom class 22 keep today's fallback, and makeverifyAtomicReplacement(and theCacheFileReadTestscase) skip with a message when the POSIX rename is unsupported, with a debt entry for cache directories on SMB shares.@MarcZX10, which one would you accept? Option 1 changes no product behaviour. I can open either as a PR, with the loopback-share run and the NTFS control in Tested.
AI-assisted: yes (Claude Code)
[-]Windows: shader disk-cache retained-reader replacement fails on tested NAS share, passes on NTFS[/-][+]Windows SMB: shader disk-cache retained-reader replacement fails; local NTFS passes[/+]MarcZX10 commentedon Oct 26, 2026
I prefer option 1 for now: preserve atomic replacement semantics, skip the retained-reader replacement case only when the POSIX rename class is specifically unsupported (
ERROR_INVALID_PARAMETER), and record that cache directories on those SMB paths cannot update entries that another process still has open.The rename-aside sequence creates a window where the published cache path is missing and can leave it missing after a crash. Since the cache is recoverable, that may be acceptable as a separately chosen behavior, but it weakens the atomicity the current helper and test are asserting. I would keep the production fallback unchanged until that tradeoff is explicitly accepted. Please keep the skip narrow to the identified unsupported operation; other replacement failures should still fail the test.
test(shader): skip open-entry cache replacements where SMB has no POS…
test(shader): skip open-entry cache replacements where SMB has no POS…
test(shader): skip open-entry cache replacements where SMB has no POS…
test(shader): skip open-entry cache replacements where SMB has no POS…