Repository navigation
fix(daemon): guard socket unlinking on shutdown by checking bound node identity - #291
Merged
Merged
Conversation
georgeh0
added a commit
that referenced
this pull request
Oct 6, 2026
ruff format and end-of-file fixes in the daemon socket guard and its test. Every Pre-commit CI job on main failed at the lint step, which also kept pytest from running for any PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
georgeh0
added a commit
that referenced
this pull request
Oct 6, 2026
It never ran daemon code: it re-implemented the (st_dev, st_ino) comparison on touch()ed files. On ext4 the replacement file reuses the unlinked inode number, so its premise fails and the test fails on ubuntu. #300 removes it too and replaces the guard it was meant to cover. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
georgeh0
added a commit
that referenced
this pull request
Oct 6, 2026
…rides as memo arg (#299) * fix(indexer): fingerprint chunkers at import time, pass overrides as memo arg #287 hashed each chunker's source file from disk on every index run, but the daemon runs the code it imported at startup. Editing a chunker and indexing before a restart stored the old code's chunks under the new fingerprint, and they stayed after the restart. A functools.partial or callable instance fell back to repr(), whose memory address re-indexed everything on every restart. - The daemon records a fingerprint per suffix when it imports a chunker: the "module:attr" spec plus a sha256 of the module file. The digest is kept per loaded module, so a project loaded again in the same daemon (ccc reset, a second project) gets the digest of the code that actually runs. Fingerprints reach process_file through a new CHUNKER_FINGERPRINTS context key with detect_change=True; CHUNKER_REGISTRY is unchanged. - indexer_main builds the suffix -> language map once per run and passes it to process_file, which uses it; the per-file load_project_settings is gone. - Remove chunking_fingerprint, _chunker_source_digest and the unused chunking_config argument. - Move the duplicated test stub embedder into a conftest fixture. - README: any chunking change re-chunks all files; unchanged chunk text keeps its cached embedding; only the chunker's own module file is hashed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(project): pass each chunker with its fingerprint Project.create took the chunker functions and their fingerprints as two dicts keyed by the same suffixes, so a caller could register chunkers without fingerprints and silently keep stale chunks. It now takes one {suffix: LoadedChunker(fn, fingerprint)} mapping and provides both context keys from it; _resolve_chunker_registry returns that mapping directly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(chunking): make CHUNKER_REGISTRY itself change-detected Instead of a separate CHUNKER_FINGERPRINTS context key that process_file read only to record a dependency, CHUNKER_REGISTRY now holds LoadedChunker values and has detect_change=True. LoadedChunker.__coco_memo_key__ returns (spec, module_sha256) and never the function: cocoindex fingerprints a function by module and name only and cannot fingerprint lambdas or closures. CHUNKER_REGISTRY's value type changes from dict[str, ChunkerFn] to dict[str, LoadedChunker]; LoadedChunker is exported from chunking. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(chunking): write exact-content test files as bytes On Windows, write_text turns "\n" into "\r\n", so the chunk contents the tests compare exactly came back as "hello\r\n". This already failed the Windows CI job on main after #287. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * style: fix lint left on main by #291 ruff format and end-of-file fixes in the daemon socket guard and its test. Every Pre-commit CI job on main failed at the lint step, which also kept pytest from running for any PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(daemon): remove #291's socket inode test It never ran daemon code: it re-implemented the (st_dev, st_ino) comparison on touch()ed files. On ext4 the replacement file reuses the unlinked inode number, so its premise fails and the test fails on ubuntu. #300 removes it too and replaces the guard it was meant to cover. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
georgeh0
added a commit
that referenced
this pull request
Oct 6, 2026
…socket Completes the fix for #288 started in #291. The inode guard added there never ran first: multiprocessing's SocketListener registers an os.unlink finalizer at bind time and runs it unconditionally in close() (shutdown step 1), so a displaced daemon still deleted the replacement's socket. - On POSIX the daemon now owns its listening socket (_UnixListener) instead of using multiprocessing.connection.Listener. It records the socket file's identity at bind and on close() unlinks the path only if it still has that identity, before closing the socket (an open socket pins its inode). close() also calls shutdown(), which is what wakes a blocked accept() on Linux; previously a displaced daemon there waited out the 5 s accept-thread join. Windows keeps Listener (named pipes). - The identity is (st_dev, st_ino, st_ctime_ns): ext4/overlayfs hand a freed inode number straight to the next file, so (dev, ino) alone can match a replacement's socket once the original is gone. - socket_identity() / unlink_socket_if_owned() in _daemon_paths are the single implementation, also used by the client's stop_daemon() cleanup, which unlinked the socket path unconditionally (same class of bug). - Replace the tautological test with an end-to-end in-process daemon test that fails on main, plus a client stop_daemon() test that covers inode reuse. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
georgeh0
added a commit
that referenced
this pull request
Oct 6, 2026
…socket Completes the fix for #288 started in #291. The inode guard added there never ran first: multiprocessing's SocketListener registers an os.unlink finalizer at bind time and runs it unconditionally in close() (shutdown step 1), so a displaced daemon still deleted the replacement's socket. - On POSIX the daemon now owns its listening socket (_UnixListener) instead of using multiprocessing.connection.Listener. It records the socket file's identity at bind and on close() unlinks the path only if it still has that identity, before closing the socket (an open socket pins its inode). close() also calls shutdown(), which is what wakes a blocked accept() on Linux; previously a displaced daemon there waited out the 5 s accept-thread join. Windows keeps Listener (named pipes). - The identity is (st_dev, st_ino, st_ctime_ns): ext4/overlayfs hand a freed inode number straight to the next file, so (dev, ino) alone can match a replacement's socket once the original is gone. - socket_identity() / unlink_socket_if_owned() in _daemon_paths are the single implementation, also used by the client's stop_daemon() cleanup, which unlinked the socket path unconditionally (same class of bug). - Replace the tautological test with an end-to-end in-process daemon test that fails on main, plus a client stop_daemon() test that covers inode reuse. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
georgeh0
added a commit
that referenced
this pull request
Oct 6, 2026
…socket (#300) Completes the fix for #288 started in #291. The inode guard added there never ran first: multiprocessing's SocketListener registers an os.unlink finalizer at bind time and runs it unconditionally in close() (shutdown step 1), so a displaced daemon still deleted the replacement's socket. - On POSIX the daemon now owns its listening socket (_UnixListener) instead of using multiprocessing.connection.Listener. It records the socket file's identity at bind and on close() unlinks the path only if it still has that identity, before closing the socket (an open socket pins its inode). close() also calls shutdown(), which is what wakes a blocked accept() on Linux; previously a displaced daemon there waited out the 5 s accept-thread join. Windows keeps Listener (named pipes). - The identity is (st_dev, st_ino, st_ctime_ns): ext4/overlayfs hand a freed inode number straight to the next file, so (dev, ino) alone can match a replacement's socket once the original is gone. - socket_identity() / unlink_socket_if_owned() in _daemon_paths are the single implementation, also used by the client's stop_daemon() cleanup, which unlinked the socket path unconditionally (same class of bug). - Replace the tautological test with an end-to-end in-process daemon test that fails on main, plus a client stop_daemon() test that covers inode reuse. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.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
2 participants
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.
Fixes #288
Description
A daemon unlinked
daemon.sockon shutdown without checking whether the file at that path was still its own. As a result, when a stale or displaced daemon was killed/shut down, it unconditionally removed the replacement/live daemon's socket file. The live daemon would continue running but become unreachable from subsequent client invocations.Changes
src/cocoindex_code/daemon.py, record the bound socket's device and inode identity (st.st_dev,st.st_ino) immediately after creating theListener.(current_st.st_dev, current_st.st_ino) == bound_sock_statbefore unlinkingsock_path(mirrors the PID file ownership check).tests/test_daemon.pyverifying that a displaced daemon shutting down does not delete a replacement daemon's socket.