mirror of
https://github.com/semantica-agi/semantica.git
synced 2026-08-29 04:26:20 +00:00
manjunath bhaskar
80b9bea0d5
* fix(ingest): lock the repo host DNS resolve cache against concurrent mutation
_REPO_HOST_RESOLVE_CACHE is a module level OrderedDict shared by every
RepoIngestor instance and thread. _resolve_repo_host_ips and
_prune_repo_host_resolve_cache read, wrote, and iterated it with no lock,
so concurrent ingest_repository() calls (e.g. from a thread pool) could
mutate the dict while another thread was iterating it during pruning.
This reliably raised RuntimeError: OrderedDict mutated during iteration
under ordinary concurrent usage, not just adversarial input.
Reproduced with 32 threads hammering _resolve_repo_host_ips with a low
TTL and small cache cap so pruning and eviction happen on nearly every
call; the crash showed up within the first few hundred iterations on
every run before the fix and did not reproduce at all after it.
Fix adds a threading.Lock guarding every read, write, and prune of the
cache. The blocking socket.getaddrinfo call stays outside the lock so a
slow DNS lookup for one host cannot stall cache access for other hosts.
Added a regression test, TestRepoHostResolveCacheThreadSafety, that
drives 32 threads through _resolve_repo_host_ips with a short TTL and
small cache cap and asserts no exception is raised.
Full test suite: 4088 passed, 332 failed, 140 errors both before and
after this change (same counts on main), all from missing optional
dependencies in this local environment (snowflake, sqlite-vec, spaCy
models, faiss/torch version mismatches), not from this fix. The ingest
and SSRF focused test files pass cleanly: 106 passed, 0 failed.
* test(ingest): fail fast on the first hung thread in the resolve-cache race test
join(timeout=30) alone doesn't fail the test if a worker hangs -- it
just returns after the timeout with the thread still running, and the
test falls through to the errors check, which trivially passes since
a hung thread never got far enough to append one. A future deadlock
could slip past this test looking green.
Assert immediately after each individual join rather than after the
whole loop: checking only once every thread has been joined means a
mass hang costs up to 32*30s = 16 minutes before the test even reaches
the check. Failing on the first hung thread caps the worst case at
~30s instead. Worker threads are daemon=True so a genuine hang can't
also block the test process from exiting.
Verified the assertion is load-bearing, not cosmetic: temporarily
injected an artificial 9999s sleep into the first worker in a
throwaway copy of the test and confirmed the test now fails in ~31s
with a clear message, instead of the ~16 minutes a mass hang would
otherwise cost. That copy was never committed.
Addresses the review comment on #979 from ZohaibHassan16 and Qodo's
automated review.
* test(ingest): fail fast on the first hung thread, for real this time
The previous commit (f94e3b38) claimed to check is_alive() right after
each individual join, but a git staging mistake meant it actually
committed the old batched version instead (checking all 32 threads
only after the whole join loop finished) -- ZohaibHassan16 caught this
by timing it directly, 5 hanging threads took ~5x longer than 1
hanging thread, which the per-thread version would not do.
This commit was built by resetting to the current branch tip, verifying
byte-for-byte against a separately saved copy of the intended fix, and
confirming the actual committed git object (not just `git diff`) has
the inline check before pushing anything.
Assert immediately after each individual join rather than after the
whole loop: checking only once every thread has been joined means a
mass hang costs up to 32*30s = 16 minutes before the test even reaches
the check. Failing on the first hung thread caps the worst case at
~30s regardless of how many threads hang.
---------