Write config temp files next to the target so the move stays atomic - #6630
Open
dekrom wants to merge 1 commit into
Open
Write config temp files next to the target so the move stays atomic#6630dekrom wants to merge 1 commit into
dekrom wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of change
Description
System#savewrites to a temp file and then moves it over the target, which is the right shape,but the temp file is created with no directory argument:
so it lands in
java.io.tmpdirwhile the target is under the game directory. When those aredifferent filesystems the atomic move cannot work,
Files.movethrowsAtomicMoveNotSupportedException, and the fallback isStreamUtils.copy, which opensnew FileOutputStream(to)and truncates the live config to zero before it writes a byte. Italso swallows the
IOExceptioninto a log line, so a failure there leaves an empty file andsave()reports nothing./tmpbeing its own tmpfs mount is the systemd default, and on Windows%TEMP%is often on adifferent drive to the instance, so for a lot of people this is not the rare fallback, it is
every single save. The temp file buys nothing there. Alt-F4 during the shutdown hook, or a full
disk, and
modules.nbtcomes back empty.Resolving the destination before creating the temp file next to it puts the move back on one
filesystem so it stays atomic. The copy fallback stays for anything exotic. The delete moved
into a
finallyas well, since a throw fromNbtIo.writeor the move used to leave the tempfile behind.
Related issues
None that I found.
How Has This Been Tested?
Confirmed on my own machine that
/tmpis a separate tmpfs mount from the game directory, whichis what sends every save down the truncating fallback. I have not gone as far as killing the
process mid-save to produce a zeroed config. Builds clean against current master.
Checklist: