Skip to content

shim: always clear the reentrancy guard in rmdir - #6

Closed
davised wants to merge 2 commits into
edaywalid:mainfrom
davised:fix/rmdir-guard
Closed

shim: always clear the reentrancy guard in rmdir#6
davised wants to merge 2 commits into
edaywalid:mainfrom
davised:fix/rmdir-guard

Conversation

@davised

@davised davised commented Jul 29, 2026

Copy link
Copy Markdown

rmdir() sets the thread-local in_shim guard on entry but clears it only when the rmdir both succeeds and gets journaled:

int rc = real_rmdir(path);
if (rc == 0 && ok) {
    in_shim = 1;
    jwrite("rmdir", abs, mode, NULL);
    in_shim = 0;      /* only reset here */
}
return rc;

After an ENOTEMPTY, or an rmdir of a path on the ignore list, the flag stays set for the rest of the process. armed() then returns false and the shim silently records nothing further — while the process carries on changing files.

Repro. One compiled program, since in_shim is thread-local and rmdir(1)/rm(1) are separate processes:

#include <unistd.h>
int main(int c, char **v) { (void)c; rmdir(v[1]); return unlink(v[2]); }

With v[1] a non-empty directory the rmdir fails, and the following unlink is never journaled. The file is gone and undo reports "nothing to undo".

Scope. Only reachable through the plain rmdir() wrapper. Anything using unlinkat(AT_REMOVEDIR) — coreutils rm -rf, Python's shutil.rmtree, git clean — is unaffected, because unlinkat() clears the guard correctly. That is why the headline rm -rf path never showed it. Direct rmdir() callers (os.rmdir, C programs, some build tooling) are affected.

The ignore-list variant is quieter: an rmdir of a .git or node_modules subdirectory succeeds but sets ok = 0, so the guard leaks with no error anywhere.

Fix. Clear in_shim unconditionally before returning. The inner assignments around jwrite are dropped as redundant — the flag is already set there, and that redundancy is what made the missing reset easy to miss.

Test. Adds e2e case 21 per CONTRIBUTING; it fails on main and passes with the fix. make test green, shim's glibc floor unchanged at GLIBC_2.34.

🤖 Generated with Claude Code

davised and others added 2 commits July 28, 2026 22:57
rmdir set in_shim on entry but cleared it only when the rmdir both
succeeded and was journaled. After an ENOTEMPTY, or an rmdir of a path on
the ignore list, the flag stayed set for the rest of the process: armed()
returned false and every subsequent change went unrecorded while still
happening on disk.

A program that calls a failing rmdir() and then unlink() left an empty
journal and an unrecoverable file. Callers going through
unlinkat(AT_REMOVEDIR) were unaffected, which is why coreutils rm -rf
never showed it.

The inner in_shim assignments around jwrite are dropped as redundant --
the flag is already set at that point, and that redundancy is what
disguised the missing reset.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@davised

davised commented Jul 30, 2026

Copy link
Copy Markdown
Author

Closing — this turns out to be a duplicate. The same guard leak was fixed independently in 91aa609 ("Stop the shim silently going deaf mid-command"), released in v0.2.8, so the shim diff here is now empty against main.

For what it is worth, I hit this from the other direction: a program calling a failing rmdir() and then unlink() in the same process left an empty journal and an unrecoverable file, while coreutils rm -rf was unaffected because unlinkat(AT_REMOVEDIR) clears the guard correctly. Good to see it already handled, along with the dedup table persisting across sessions under UNDO_CAPTURE_SHELL=1 — that second one is a nastier failure than the first and I had read straight past it.

Thanks for the tool.

@davised davised closed this Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant