Stop the shim silently going deaf mid-command#3
Open
lai0xn wants to merge 1 commit into
Open
Conversation
Two paths made the shim stop journaling without any sign to the user, which is the worst failure mode this tool has: `undo` reports "nothing to undo" for changes it should have captured. rmdir() set the reentrancy guard on entry but only cleared it on the success path, so any failed rmdir (a non-empty directory, ENOENT) left in_shim set for the rest of the process. Everything that process did afterwards saw armed() == false and went unrecorded. `rmdir a b` with a non-empty `a` was enough to lose `b`. The mod dedup table was keyed on nothing, so it persisted across sessions in a process that outlives one command -- exactly the UNDO_CAPTURE_SHELL=1 setup, where the shim lives in the shell itself. A file written in one command suppressed its own backup in every later one. Reset the table when the session directory changes. Also propagate signal deaths from `undo run` as 128+signal instead of letting ExitCode()'s -1 become a bare 255.
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
1 participant
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.
Two bugs where the shim stops journaling with no sign to the user, so
undosays "nothing to undo" for changes it should have captured.rmdir()leaks the reentrancy guard. It setsin_shim = 1on entry but only clears it on the success path, so any failed rmdir leaves it set for the rest of the process — every later unlink/open/rename seesarmed() == falseand goes unrecorded.rmdir a bwith a non-emptyais enough to loseb. Every other interposed function clears it on all paths.The mod dedup table never resets between sessions. It's process-global with no session key. Fine for a fresh child per command, wrong under
UNDO_CAPTURE_SHELL=1where the shim lives in the shell across every command: a file backed up once is skipped forever after, so later edits are unrecoverable. Now keyed on the session dir.Minor:
undo runreturned 255 for signal deaths (ExitCode()is -1); now 128+signal.e2e cases 19 and 20 both fail on the old shim. Case 20 uses python3 to drive the one-process-two-sessions path, since bash's
exportdoesn't reliably update the Cenvironthe shim reads; it skips where python3 is absent so the musl job doesn't break.