Skip to content

fix(agc): remove the temporary benchmark file when its write fails - #12234

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
MarcZX10:fix/trace-file-remove-temporary-on-write-failure
Oct 28, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
MarcZX10:fix/trace-file-remove-temporary-on-write-failure

Conversation

@MarcZX10

Copy link
Copy Markdown
Contributor

What

TraceFile::ReplaceGzip writes the gzip to <name>.tmp and renames it over the benchmark file. The write sat outside any handler, so a write that failed half way (a full disk, for example) threw out of ReplaceGzip and left the partial temporary file behind; only a failed rename removed it. FlushInterval calls it every interval, so the stale file stays until a later write truncates it.

The write is now guarded: the temporary file is removed (unless something else made that name a directory) and the exception is rethrown.

Two commits: the fix, then the test (ReplaceGzipFile in PerformanceTrace.cpp: on Linux, <name>.tmp is a symlink to /dev/full, which accepts the open and fails the write with ENOSPC; ReplaceGzip has to throw and leave no temporary file).

Tested

Linux x86-64, a throwaway archive build with the host CPU gate patched. agc_performance_trace passes. With the guard removed the same test fails with "a replace whose write failed left its temporary file behind". The case is skipped when /dev/full does not exist and does not run on Windows. tools/check_conventions.py --base upstream/main and git diff --check pass.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

ReplaceGzip writes the gzip to <name>.tmp and renames it over the benchmark
file. The write sat outside any handler, so a write that failed half way, such
as a full disk, threw out of ReplaceGzip and left the partial temporary file;
only a failed rename removed it. The write is now guarded: the temporary file
is removed (unless something else made it a directory) and the exception is
rethrown.
<name>.tmp is a symlink to /dev/full, which accepts the open and fails the write
with ENOSPC; ReplaceGzip has to throw and leave no temporary file. The case
runs on Linux when /dev/full exists.
@maskym

maskym commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Windows + NVIDIA result for head 5e96d65b83, merged into main 3b979d42a9 (the PR's GPUs tested line names no NVIDIA run on Windows).

This PR's own tests: agc_performance_trace (0.16 s), all passed.

Host: Windows 11 26300, Intel i7-12700K, NVIDIA GeForce RTX 2070 SUPER (vendor 0x10de, device 0x1e84; Vulkan loader 1.4.357 from the NVIDIA driver 621.01), Khronos Validation Layers nightly 1.4.366 (b2b44c0d37) through VK_ADD_LAYER_PATH, WinLibs GCC 15.2.0 (x86_64-ucrt-posix-seh, r6), Release, BUILD_TESTING=ON, Ninja. Build clean.

Same focused run on main 3b979d42a9 and on this tree, same machine and GPU: VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, ANYPS5_NO_SHADER_CACHE=1, ANYPS5_REQUIRE_VULKAN=1, VK_LOADER_DEBUG=error,warn,layer, ctest --test-dir build -R '^(agc|guest_video_out|video_out)' --parallel 4 -V.

The tree also carried the open PRs #12298, #12272, #12262, #12256, #12250, #12228, #12211, #12210, #12209, #12208, #12207, all merged cleanly, so the run is a joint one. Not run on a PS5.

AI-assisted: yes (Claude Code), the runs and the numbers above are from this machine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants