Skip to content

Robustify file renaming#4721

Merged
WeiqunZhang merged 1 commit intoAMReX-Codes:developmentfrom
WeiqunZhang:robust_rename
Oct 20, 2025
Merged

Robustify file renaming#4721
WeiqunZhang merged 1 commit intoAMReX-Codes:developmentfrom
WeiqunZhang:robust_rename

Conversation

@WeiqunZhang
Copy link
Copy Markdown
Member

In amrex::UtilCreateCleanDirectory, if a directory already exists, we preserve it by renaming it to a directory whose name includes a string generated by amrex::UniqueString(). However, this string is not guaranteed to be unique. The behavior of std::rename is implementation-defined, and it may fail if the destination already exists. This happens rarely, but it does happen.

This PR addresses the issue by removing the existing conflicting directory before renaming. This makes the file renaming process more robust.

In `amrex::UtilCreateCleanDirectory`, if a directory already exists, we
preserve it by renaming it to a directory whose name includes a string
generated by `amrex::UniqueString()`. However, this string is not guaranteed
to be unique. The behavior of `std::rename` is implementation-defined, and
it may fail if the destination already exists. This happens rarely, but it
does happen.

This PR addresses the issue by removing the existing conflicting directory
before renaming. This makes the file renaming process more robust.
Copy link
Copy Markdown
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you :)

Should TryFileOutput have a BL_PROFILE, too?

@WeiqunZhang
Copy link
Copy Markdown
Member Author

I thought about TryFileOutput, but decided not to add profiler to it. The file names there are different, and it's even less likely to have conflicts.

@WeiqunZhang WeiqunZhang merged commit 2e583c2 into AMReX-Codes:development Oct 20, 2025
73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants