Fix Windows temp-file cleanup failing on paths over MAX_PATH by alexreinking · Pull Request #9452 · halide/Halide

alexreinking · 2026-09-15T18:54:47Z

Summary

`compile_multitarget()` names per-subtarget temp object files by concatenating the output prefix with the full, unelided subtarget suffix (every feature name spelled out), which can push the absolute path past the legacy Windows `MAX_PATH` limit (260 chars) once combined with a long temp directory path.
`_unlink()`/`RemoveDirectoryW()` then silently fail for that one file (no error checking on the unlink side), so `TemporaryFileDir`'s destructor goes on to call `RemoveDirectoryW()` on a directory that still isn't actually empty, raising `error 145` (`ERROR_DIR_NOT_EMPTY`).
Root-caused via a live repro: at the moment of failure, Sysinternals `handle64.exe` found no process anywhere on the system holding the file open (ruling out a lock/AV/indexer race), but the failing path was exactly 261 characters versus 245 for a sibling file that always deleted fine -- squarely the `MAX_PATH` boundary.
Fixed by opting both `file_unlink()` and `dir_rmdir()` out of `MAX_PATH` via the well-known `\?\` long-path prefix (which requires an absolute, backslash-separated path, so the existing forward-slash paths are converted first).

Test plan

Reproduced the failure reliably on Windows with `test/correctness/compile_to_multitarget.cpp` (previously failing ~90% of runs)
Confirmed 15/15 clean runs after the fix
Re-ran `correctness_struct_type`, `correctness_parallel`, `correctness_async`, `correctness_make_struct`, `correctness_thread_safety` to confirm no regressions from the path-handling change

compile_multitarget() names per-subtarget temp object files by concatenating the output prefix with the full, unelided subtarget suffix (every feature name), which can push the absolute path past the legacy Windows MAX_PATH limit (260 chars) once combined with a long temp directory path. _unlink()/RemoveDirectoryW() then silently fail for that one file, and TemporaryFileDir's destructor -- which doesn't check file_unlink()'s return value -- goes on to call RemoveDirectoryW() on a directory that still isn't actually empty, raising "error 145" (ERROR_DIR_NOT_EMPTY). Root-caused via a live repro: at the moment of failure, Sysinternals handle64.exe found no process anywhere on the system holding the file open (ruling out a lock/AV/indexer race), but the failing path was exactly 261 characters versus 245 for a sibling file that always deleted fine -- squarely the MAX_PATH boundary. Fixed by opting both file_unlink() and dir_rmdir() out of MAX_PATH via the well-known `\?\` long-path prefix (which requires an absolute, backslash-separated path, so the existing forward-slash paths are converted first). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

abadams · 2026-09-15T19:08:14Z

Isn't this going to hit other places we access the filesystem too? It just uses the new function for a single call.

alexreinking · 2026-09-15T19:16:11Z

Isn't this going to hit other places we access the filesystem too? It just uses the new function for a single call.

Likely yes, but this fixes the one issue I encountered and could reproduce locally. It would be worth an audit of the other _<posix> filesystem functions we use on Windows.

abadams · 2026-09-15T19:41:51Z

Other potential issues in the same file are the calls to _access, _stat, and maybe ifstream, ofstream, CreateDirectoryW, and GetTempFileNameW

Shouldn't we just wrap all the filename arguments to all file-system related calls inside WIN32 blocks?

slomp · 2026-09-15T20:16:12Z

FWIW, prefixing a Windows path with \\?\ will bypass the 260 MAX_PATH limit.
https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry

File I/O functions in the Windows API convert "/" to "" as part of converting the name to an NT-style name, except when using the "\?" prefix as detailed in the following sections.

alexreinking · 2026-09-15T20:24:25Z

FWIW, prefixing a Windows path with \\?\ will bypass the 260 MAX_PATH limit.

That's how this PR works

mcourteaux · 2026-09-15T20:51:57Z

Windows is ridiculous. We're dealing with limitations introduced 33 years ago... 😞

codecov · 2026-09-16T10:22:48Z

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 69.98%. Comparing base (d33fd25) to head (6e9c2a9).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
src/Util.cpp	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9452      +/-   ##
==========================================
- Coverage   70.21%   69.98%   -0.23%     
==========================================
  Files         261      261              
  Lines       79792    79940     +148     
  Branches    19451    19478      +27     
==========================================
- Hits        56022    55945      -77     
- Misses      17966    18086     +120     
- Partials     5804     5909     +105

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

_waccess(), _stat(), _open(), and MSVC's <fstream> all normalize the path via GetFullPathName() internally before handing it to the kernel, and that function doesn't understand the \\?\ long-path prefix -- so wrapping their paths in to_long_path() breaks them instead of fixing anything (this is what made correctness_run_process fail on Windows: _wopen() was returning -1 on a to_long_path()'d temp file). to_long_path() remains in use for the raw Win32 APIs it was introduced for (DeleteFileW, RemoveDirectoryW, CreateDirectoryW), which do support the prefix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…/write_entire_file/run_process The previous commit reverted these to plain CRT calls (_access, _stat, <fstream>, _open) to fix a Windows CI regression: to_long_path()'s \\?\ prefix only works with raw Win32 file APIs, not with CRT path functions, which normalize the path via GetFullPathName() internally and don't understand the prefix. This restores the intended long-path support at each site, but via the raw Win32 equivalents instead: GetFileAttributesW for file_exists(), GetFileAttributesExW for file_stat(), and CreateFileW()+ReadFile()/WriteFile() for read_entire_file()/ write_entire_file(). run_process() additionally needs the resulting handle to be inherited by the spawned child, so its helper explicitly requests an inheritable handle (CreateFileW() defaults to non-inheritable, unlike _open()) and wraps it in a CRT fd via _open_osfhandle() so the existing _dup2()/_fileno() redirection dance keeps working unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…LE leaks, file_make_temp, symlinks - to_long_path() misclassified drive-relative paths like "C:foo.txt" as absolute (only checked for "X:", not "X:\"), corrupting them with the \\?\ prefix; also never handled UNC paths, which need \\?\UNC\ instead. Fixed the classification and added \\?\UNC\ support. This also makes the old "doesn't already have it" idempotency check redundant (an already-prefixed path starts with a backslash, so it could never match either absolute-drive or UNC classification anyway) -- removed it. - read_entire_file()/write_entire_file() leaked their CreateFileW HANDLE whenever a later internal_assert() threw (e.g. a failed ReadFile/ WriteFile mid-loop), since CloseHandle() was only reached on the success path. Added an AutoHandle RAII wrapper so the handle closes on every exit path, and reused it in file_stat() below. - file_make_temp() was left calling GetTempFileNameW(), whose output buffer is a fixed-size WCHAR[MAX_PATH] with no long-path form at all, so it still hit the exact MAX_PATH failure this branch exists to fix. Replaced it with the same CoCreateGuid()-based approach dir_make_temp() already uses for directories. - file_stat() used GetFileAttributesExW(), which reports a reparse point's own attributes rather than following it, unlike _stat(). Switched to CreateFileW() (follows reparse points by default) + GetFileInformationByHandle(). Also reworded the comments at each of these sites to describe the current design rather than what they replaced, per this repo's commenting conventions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

alexreinking requested review from derek-gerstmann and slomp September 15, 2026 19:16

alexreinking requested a review from mcourteaux September 15, 2026 20:56

Clean up comments

9e5ab03

derek-gerstmann approved these changes Sep 15, 2026

View reviewed changes

alexreinking and others added 4 commits September 16, 2026 16:22

Use to_long_path in more places.

baa9af0

Navigation Menu

Fix Windows temp-file cleanup failing on paths over MAX_PATH - #9452

Fix Windows temp-file cleanup failing on paths over MAX_PATH#9452
alexreinking wants to merge 6 commits into
mainfrom
alexreinking/fix-windows-max-path-cleanup

alexreinking commented Sep 15, 2026

Uh oh!

abadams commented Sep 15, 2026

Uh oh!

alexreinking commented Sep 15, 2026

Uh oh!

abadams commented Sep 15, 2026

Uh oh!

slomp commented Sep 15, 2026

Uh oh!

alexreinking commented Sep 15, 2026

Uh oh!

mcourteaux commented Sep 15, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Sep 16, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

alexreinking commented Sep 15, 2026

Summary

Test plan

Uh oh!

abadams commented Sep 15, 2026

Uh oh!

alexreinking commented Sep 15, 2026

Uh oh!

abadams commented Sep 15, 2026

Uh oh!

slomp commented Sep 15, 2026

Uh oh!

alexreinking commented Sep 15, 2026

Uh oh!

mcourteaux commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mcourteaux commented Sep 15, 2026 •

edited

Loading

codecov Bot commented Sep 16, 2026 •

edited

Loading