fix(ui): unwatch projects on UI delete; harden deferred-free OOM corner#843
Merged
Conversation
DELETE /api/project removed the project's .db file but never told the watcher; the zombie watch kept polling the repo and the next change silently resurrected the deleted DB (#803). Wire the watcher into the UI HTTP server (non-owned ref; created before the server, freed after it) and unwatch after a successful unlink and on ENOENT (clears an already-orphaned watch, still 404). Invalid name stays a plain 404 with no unwatch; a non-ENOENT unlink failure stays 500 and keeps the watch. The cbm_file_exists pre-check is replaced by unlink-errno handling, removing the TOCTOU window. Distilled from #830, rebased over the defer_state_free refactor (#817): the OOM hardening of the deferred-free path moves into defer_state_free itself so BOTH unwatch and stale-root prune get it. On pending-free realloc failure the helper no longer frees the state immediately (a potential use-after-free against an in-flight poll_once snapshot) — it logs watcher.unwatch.oom and returns failure, and the callers keep the entry registered; the prune path retries on the next poll cycle. Carries six httpd regression tests from #830 driving the live UI server with a real watcher wired in. Closes #803. Co-authored-by: Dustin Persek <dustin.persek@protonmail.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
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.
fix(ui): unwatch projects on UI delete; harden deferred-free OOM corner
Distilled from #830 (author: Dustin Persek dustin.persek@protonmail.com), rebased over the
defer_state_freerefactor that landed with #817.Summary
DELETE /api/projectremoved the project's.dbfile but never told the watcher. The zombiewatch kept polling the repo and the next change reindexed it — silently resurrecting the DB the
user just deleted (#803).
cbm_http_server_set_watcher) right afterserver creation. Lifecycle holds on current main: watcher is created (l.658) before the HTTP
server (l.677); teardown stops/frees the HTTP server (l.702–705) before the watcher (l.709–712),
so the non-owned reference can never dangle.
handle_delete_projectnow unwatches after a successful unlink and onENOENT(clears an already-orphaned zombie watch, still 404). Invalid project name → 404 withno unwatch. Unlink failure (non-ENOENT) → 500 and the watch is kept. The
cbm_file_existspre-check is replaced by unlink-errno handling (removes the TOCTOU window).
pending_freelist failed, the state was freed immediately — a potential use-after-free againstan in-flight
poll_oncesnapshot.defer_state_freenow returns failure without freeing;callers keep the entry registered (
cbm_ht_deleteonly happens once the state is safely on thedeferred-free list) and emit
watcher.unwatch.oom.Re-target rationale (why this differs from #830's watcher hunk)
#830 was audited CORRECT but written against pre-#817 code, where the pending-free logic lived
inline in
cbm_watcher_unwatch. Main has since extracted it intodefer_state_free, shared bycbm_watcher_unwatchand the stale-root prune path (prune_missing_project) — and the UAF-pronestate_freefallback moved inside that helper. Applying #830's hunk verbatim would have hardenedonly the unwatch path and left the same bug reachable via root-prune. The hardening therefore goes
into
defer_state_freeitself (fail → keep registered → warn), covering both call sites; theprune path additionally retries on the next poll cycle.
Tests (carried from #830, adapted to current main)
Six regressions in
tests/test_httpd.c(suitehttpd), driving the live UI server over a socketwith a real watcher wired in:
ui_server_delete_project_unwatches_after_delete(core red-first case)ui_server_delete_project_unwatches_missing_db(ENOENT zombie-clear, still 404)ui_server_delete_project_no_watcher_still_deletesui_server_delete_project_missing_name_keeps_watch(400)ui_server_delete_project_invalid_name_keeps_watch(404, no unwatch)ui_server_delete_project_unlink_failure_keeps_watch(500)Red/green evidence
RED —
src/ui/http_server.c+src/main.creverted toorigin/main(plus a link-only no-opcbm_http_server_set_watchershim so the fixture compiles), rebuilt,test-runner httpd:GREEN — fix restored, rebuilt,
test-runner httpd watcher:make -f Makefile.cbm cbmbuilds -Werror clean;make -f Makefile.cbm lint-cipasses.Closes #803.