已关闭
[Feedback/bug][cache] Concurrent local Go gate processes can collide on the fixture service fallback writer lock #166
urandon创建于  18 天前关闭于  18 天前
urandon
urandon成员
18 天前 创建

Feedback summary

Concurrent local Go gate processes can collide on the fixture service fallback writer lock

Classification

  • Category: bug
  • Surface: cache
  • Reporter: agent

Observed behavior

During #150 verification, overlapping full uncached Go and Admin race gate processes caused TestDurableOfflineFixtureIssueBatchCommits and TestDurableIssueBatchFetchDoesNotHoldTargetWriterAndCommitDoesNotRefetch to fail with cache.ErrLockContention. Holder operations named unrelated synthetic repositories. A complete isolated service race rerun passed. serviceLockPath selects one shared temporary fallback when no explicit lock is supplied; fixture services use it despite independent in-memory stores.

Expected behavior

Independent test stores and independent gate processes should remain isolated; only deliberately shared cache authority should produce writer contention.

Impact

Parallel verification can become spuriously red and obscure real cache regressions. The initial failure must remain visible rather than being replaced by unexplained retries.

Reproduction

  1. On main 57b2359175b18c0ca08527046c05adf072b06682 or the UI-only #150 working patch, overlap go test -count=1 ./... with go test -race -count=1 ./internal/adminhttp ./internal/servicectl ./internal/cache ./internal/rag ./internal/service.
  2. Inspect service fixture bulk-batch failures for cache.ErrLockContention naming another synthetic repository.
  3. Run go test -race -count=1 ./internal/service alone and compare; this verification rerun passed in 55.268s. Timing is nondeterministic and a stable regression must still be isolated.

Workaround or fallback

Workaround: Until isolation is repaired, do not overlap independent full Go gate processes sharing the same OS temporary directory; run complete gates sequentially and retain first-failure evidence.

Sanitized runtime evidence

  • gitcode-mcp version: 0.3.4
  • commit: 64485cdad2b52830d8c6590053e695aec8377cad
  • provider mode: live
  • cache schema: 21/21 (compatible: true)
  • sink binding: configured
  • OS family: darwin
  • observed at: 2026-10-09T17:31:20Z
  • No Go service/cache runtime code is changed by #150.
  • Initial combined race command: four packages passed, internal/service failed in two bulk fixture tests with writer-lock contention.
  • Isolated full service race command passed; no flags disabled race instrumentation or internal test parallelism.
  • Source inspection: serviceLockPath has one shared temporary fallback; in-memory SQLite stores have their own ephemeral lease unless overridden by a service request.

https://gitcode.com/urandon/gitcode-mcp/issues/150

Acceptance signal

A deterministic independent-store/process regression and repeated concurrent uncached/race gate runs pass without weakening deliberate same-cache contention tests.

Proposal

Isolate default fixture service writer authority by store/test while preserving explicit shared-lock contention tests. Capture a deterministic regression before changing implementation.


Generated from structured, sanitized feedback. Do not edit this issue description after intake; add later evidence and design decisions as comments.

likedislike
urandonurandon成员
18 天前 添加了label:dogfoodfeedback
urandonurandon成员
18 天前 关联了里程碑:Bugfix and feedback wave v0.3.5
urandonurandon成员
18 天前 关联了pull request:Keep Admin maintenance target and intent synchronized
urandonurandon成员
18 天前 关联了pull request:Fix live PR comment origin and bounded targeted cache admission
urandon
urandon成员
18 天前 评论:

Recovery/design at clean main 1ea0ed0616e080e8f99efdc3dabd41301d26b02f. Exact main CI 38010323471 is completed/success; no existing issue comments or linked implementation found.

Reproduction plan: hold writer admission in one default fixture service, then commit a fetched fixture batch in a second independent store. Repeat with a child test process using a pipe readiness/release barrier, not scheduling sleeps. Capture the deterministic failure before changing production code. Preserve the reported initial #150 gate failure as evidence; isolated reruns are not the fix.

Design: an omitted service lock delegates to Store.AcquireWriter's own authority instead of the global service fallback. Explicit lock overrides remain unchanged. The private nested-bulk admission marker must be scoped to its owning service, repository and lock selection, and must cease authorizing reuse when the outer lease is released; an empty default must never make unrelated service contexts equivalent. Private in-memory SQLite lock identities will use the already-generated random database identity, not pointer addresses that are not globally unique across processes. Ordinary persistent cache paths continue to share their per-file writer authority.

Acceptance tests: independent memory and distinct file stores can progress concurrently; separate services/handles for the same store/cache and explicit shared overrides still contend; nested same-service admission reuses one active lease, but a foreign or released context cannot bypass admission; subprocess overlap is deterministic and credential/network-free. Then repeat the reported full uncached Go gate concurrently with the full Admin/cache/RAG/service race gate, retaining each first result.

Scope/non-goals: no schema migration, remote API change, broader DSN/URI canonicalization or new retry policy. CLI/MCP/Admin keep the same shared service and typed contention contracts. No new user-visible resource/action or remediation cohort is introduced, so no separate Admin task is required. Risk is weakening genuine shared-cache exclusion or leaking admission across contexts; explicit negative regressions cover both. Full local gates, independent exact-SHA review after PR, head CI and resulting main CI remain mandatory before closure.

likedislike
urandon
urandon成员
18 天前 评论:

Deterministic fail-first captured against unchanged production main 1ea0ed0616e080e8f99efdc3dabd41301d26b02f: go test -count=1 ./internal/service -run 'Test(DefaultWriter|BulkWriterAdmissionLifetime)' fails independent memory/file store commits and pipe-barrier independent memory/file child-process commits with cache.ErrLockContention. The same-file subprocess negative control passes. Separate services for the same cache, when handed the outer admission context, incorrectly return no error instead of contention; explicit shared-lock foreign-context reuse also bypasses admission. This validates both isolation and marker-scoping risks.

The first two test-harness drafts had duplicate repository initialization and child cleanup-order defects; these were corrected before the final fail-first run. They are not production evidence. The valid run has no subprocess/harness failure, only the asserted production defects. No production fix applied yet.

likedislike
urandon
urandon成员
18 天前 评论:

Implementation/author verification at f3974316ca42d1aed7ffb2ebc9bf71dc1731a122 on base 1ea0ed0616e080e8f99efdc3dabd41301d26b02f.

Passed: focused service/cache race regressions ×5 (independent stores/processes, shared-cache/explicit-lock controls, marker lifetime, private memory identity, original durable-batch regressions, targeted wait boundaries). The reported acceptance commands ran concurrently twice: go test -count=1 ./... alongside go test -race -count=1 ./internal/adminhttp ./internal/servicectl ./internal/cache ./internal/rag ./internal/service; both complete pairs passed (race service 79.318s and 79.378s). Additional full go test -race -count=1 ./internal/mcp ./internal/cli, go vet ./..., product-doc contracts and staged whitespace check passed. Initial deterministic failure remains recorded above, not replaced by unexplained retries.

The service now delegates omitted overrides to the store. Explicit shared locks and same-cache handles/processes still contend. Active nested admission is owned by one service and invalidated on release; a foreign/released context cannot bypass the writer. Private memory lock names use the already-generated 128-bit database identity; closing one store cannot unlink another's lock. No migration or provider retries, no changed Admin assets/flow. Browser suite not rerun locally for this backend-only change; exact-SHA CI remains required and includes the semantic browser gates. Independent review and CI are pending; not ready to merge.

likedislike
urandonurandon成员
18 天前 关联了pull request:Fix default writer authority isolation across caches
urandon
urandon成员
18 天前 评论:

Required exact-head CI 38011386569 completed/success on f3974316ca42d1aed7ffb2ebc9bf71dc1731a122, branch codex/issue-166-default-writer-isolation, push event, .github/workflows/ci.yml. Luna reported success; root independently fetched and verified the terminal machine-readable metadata. This includes the repository semantic Admin browser/artifact gates. GitCode PR head/base and mirror freshness were also verified.

Independent review is still in progress, including checking ordinary filesystem aliases of the same cache. No merge decision until the final verdict and any P0-P2 remediation; CI green alone is not completion.

likedislike
urandon
urandon成员
18 天前 评论:

Independent review found a blocking P2 on f3974316ca42d1aed7ffb2ebc9bf71dc1731a122: two default services opened through an ordinary cache filename and a filesystem symlink alias share the same durable cache UUID, but the second commits one record while the first holds writer admission. Reviewer deterministic race probe ×5 reproduces successful publication instead of cache.ErrLockContention. This is an ordinary filesystem path alias, not URI/DSN generalization. Green head CI is insufficient; no merge.

Design correction: canonicalize the physical file path used for the store's default writer authority. For a not-yet-created cache, canonicalize its existing parent directory so opening through directory aliases is consistent from first creation. Existing-file and directory-alias controls must preserve same-cache contention, while independent files still progress. Migration and normal store opening must choose the same default lease. Explicit caller-selected lock overrides remain unchanged; no schema change. Add fail-first author regressions, rerun complete gates and obtain independent review plus CI on the new exact SHA. Original issue input unchanged.

likedislike
urandon
urandon成员
18 天前 评论:

Reviewer P2 reproduced independently by the author: go test -race -count=1 ./internal/service -run '^TestDefaultWriterSamePhysicalCacheAliases$' failed the existing-file symlink case on unchanged f3974316ca42d1aed7ffb2ebc9bf71dc1731a122 (committed=1); directory-alias negative controls passed. Reviewer also compared the same probe to exact base and confirmed it is an introduced regression.

Correction now implemented, awaiting complete gates/new commit: physical cache paths are resolved before choosing the store lock; new files use a canonical existing parent, and a dangling final file symlink is followed with a bounded depth instead of choosing its alias filename. Resolution failures fail admission setup rather than guessing authority. Migration resolves the same physical path. Explicit lock overrides and URI handling are unchanged.

Passed focused correction race ×5: file aliases, dangling-file aliases, existing/new directory aliases, shared handles/processes, independent stores/processes, active/released/foreign markers, explicit overrides, memory identity/cleanup, composite nesting and migration contention/readmission. Full concurrent gate pairs and transport races are being rerun. No merge; independent exact-SHA re-review and CI must repeat after the correction is committed.

likedislike
urandon
urandon成员
18 天前 评论:

Full verification remains blocked: first canonical-path concurrent gate pair passed (race service 90.499s); second pair's race gate passed (95.915s) but the full uncached suite failed TestBulkSyncIssuesBoundedCancelMidway_CancelAfterPage3Progress (success_count=49, expected 30) and TestBulkSyncIssuesBoundedCancelMidway (31, expected 30). Focused uncached ×20 subsequently passed; that does not repair timing dependence. Duplicate search/intake isolated #171, now tracked on release epic #165 before further delivery.

The cancellation fixtures' buffered asynchronous progress observer cannot guarantee cancellation before page 4. #171 will retain exact partial/cancel/frontier assertions with an explicit page-boundary handshake, on a separate issue branch/PR. !162 is not ready to merge despite prior head CI success. The final physical-path correction also opens SQLite and migration backup through the selected canonical path, and adds a connection-replacement/alias-retarget control; focused verification will precede the clean pushed handoff. Resume #166 after #171 with the latest main, repeat full gates, independent review and exact-SHA CI.

likedislike
urandon
urandon成员
18 天前 评论:

Clean pushed correction head: 2a52bb4de0b5e03cd887bf1dc6cfcbf284799737, base main 1ea0ed0616e080e8f99efdc3dabd41301d26b02f, !162. Reviewer P2 correction includes canonical normal/migration writer paths, canonical SQLite opening and migration backup, new/dangling file and directory aliases, and pool connection replacement after alias retarget.

Passed on the final correction: go test -race -count=5 ./internal/cache ./internal/service -run 'Test(DefaultWriter|BulkWriterAdmissionLifetime|PrivateMemoryWriterIdentity|PhysicalCacheWriterAuthorityIncludesMigration|CanonicalCacheOpenSurvivesAliasRetarget|BulkSyncAllReuses|BulkSyncWriterAdmission)' (cache 7.293s, service 40.320s); product docs; staged whitespace check. All author Go processes ended; shared checkout clean.

Remaining gates: full final-head gates are blocked by #171's captured asynchronous cancellation fixture failure; the earlier full-pair results are not claimed for this final SHA. Exact-SHA independent re-review and CI are being requested for this correction, but neither can authorize merge around #171. Next action is the separate #171 test-only fix on main, then resume/rebase this branch and rerun full/concurrent gates/review/exact CI. No merge, issue closure or release performed.

likedislike
urandon
urandon成员
18 天前 评论:

Independent exact-SHA re-review of 2a52bb4de0b5e03cd887bf1dc6cfcbf284799737 fixed the previous same-physical-cache exclusion bypass, but found a new P2: canonicalization wraps filesystem errors whose text can disclose a private symlink target through a cached CLI read with an explicit cache override. Reviewer-only synthetic fixture: candidate race ×5 FAIL, exact base 1ea0ed0616e080e8f99efdc3dabd41301d26b02f race ×5 PASS. No private fixture paths were published. All isolation/shared-authority/migration/retarget controls passed ×5; reviewer processes ended.

Correction plan: return an opaque cache-authority resolution error with safe classification instead of embedding pathname-bearing filesystem causes in public text; add cache-boundary and CLI regression coverage. No production admission semantics or assertions will be relaxed. CI run 38012674502 was independently confirmed successful for this exact SHA, but review is not clear and #171 still blocks the full verification gate. No merge, closure or release.

likedislike
urandon
urandon成员
18 天前 评论:

Clean pushed correction / explicit non-complete handoff

!162 head is add1720dbf1cef8634094b7ad5bb129fb04122d3, base 1ea0ed0616e080e8f99efdc3dabd41301d26b02f. The new reviewer P2 is corrected by ErrWriterAuthorityResolution: fixed public message and portable sentinel classification only; no pathname-bearing cause remains reachable. Cache-boundary and CLI cached-read regressions failed before the correction without printing private coordinates. One draft mistakenly expected an absent-cache migration no-op to error; that fixture assumption was corrected to exercise the actual resolution boundary, without changing migration semantics.

Passed on the final working tree committed as this SHA: focused authority/CLI/isolation/shared/symlink/migration/retarget controls under race ×5 (cache 7.581s, CLI 1.595s, service 37.062s); go test -count=1 ./...; go test -race -count=1 ./internal/cache ./internal/service ./internal/cli ./internal/mcp; go vet ./...; ./scripts/check-product-docs.sh; git diff --check and staged whitespace check. All root Go processes ended. Independent exact-SHA re-review requested; remote CI for the new SHA is pending. The earlier successful run 38012674502 belongs to 2a52bb4, not this head.

Concurrent full-process acceptance is still BLOCKED by #171 and is not claimed passed. This branch is clean and pushed, not complete or mergeable by policy. Next: fix #171 in its separate test-only PR based on green main, then rebase this branch, repeat concurrent full/race gates and obtain review/CI for the new exact SHA. No issue closure, merge or release tag.

likedislike
urandon
urandon成员
18 天前 评论:

Independent exact-SHA review: add1720dbf1cef8634094b7ad5bb129fb04122d3 is CLEAR, no P0–P2 findings remain. Reviewer independently verified both original reproductions (same physical symlink-cache exclusion bypass and CLI symlink-target disclosure) under race ×5; authority/migration/retarget/shared/subprocess and CLI/MCP transport controls passed ×5; docs/diff passed. Remote ref matched this SHA; cached unresolved discussions: 0. Reviewer used an immutable export and ended all Go/browser/subprocess processes without touching the shared checkout.

This is correctness review evidence for this exact head only. #171 and the concurrent full-process acceptance remain blockers; a future rebase requires new exact-SHA review and CI. No merge or issue closure.

likedislike
urandon
urandon成员
18 天前 评论:

Luna CI watcher and root independent metadata read confirm exact-head CI run 38013307766 completed successfully for add1720dbf1cef8634094b7ad5bb129fb04122d3, branch codex/issue-166-default-writer-isolation, workflow CI (.github/workflows/ci.yml), event push. Correctness review is clear for this head. #171 and concurrent integrated full/race acceptance still block merge/release; earlier head results are not substituted for a later rebased SHA.

likedislike
urandonurandon成员
18 天前 关联了pull request:Make progress cancellation fixtures scheduler-independent (#171)
urandon
urandon成员
18 天前 评论:

#171 test-only !163 is merged and its exact resulting main 5a895c5055396185f494a14bc364f9dd7de59080 is green (CI 38014012065). This branch is now rebased on that main; local new head 004f54e23cbef25eb739df2250ccfb7a66ffb976. No additional production change during rebase.

Starting two repeated concurrent gate pairs: go test -count=1 ./... simultaneously with go test -race -count=1 ./internal/adminhttp ./internal/servicectl ./internal/cache ./internal/rag ./internal/service. Additional full CLI/MCP race, vet/docs/whitespace gates follow only if both pairs pass. Review/CI from add1720 is not evidence for this rebased SHA; fresh independent review and exact-SHA CI remain required before merge. #171 remains open until integrated acceptance and resulting main CI are proven.

likedislike
urandon
urandon成员
18 天前 评论:

Rebased exact head 004f54e23cbef25eb739df2250ccfb7a66ffb976 is now locally verified with merged #171. git range-diff confirms all three #166 patches are unchanged from reviewed add1720; only base is now green main 5a895c5055396185f494a14bc364f9dd7de59080.

PASSED twice consecutively: go test -count=1 ./... concurrently with go test -race -count=1 ./internal/adminhttp ./internal/servicectl ./internal/cache ./internal/rag ./internal/service (exit 0/0 for each pair). Also passed go test -race -count=1 ./internal/cli ./internal/mcp; go vet ./...; ./scripts/check-product-docs.sh; git diff --check. All root Go processes ended. No assertion weakening/excluded tests/special race flags; isolated-store/process regressions and genuine same-cache/explicit-shared controls remain enabled.

Clean rebased branch update uses force-with-lease pinned to the known previous remote SHA add1720, preserving any unexpected concurrent remote edit. Fresh independent exact-SHA review and CI are required before merge; older review/CI are not substituted. #171 integrated local acceptance is recorded, but both issues remain open until merge and resulting main CI are verified.

likedislike
urandon
urandon成员
18 天前 评论:

Final independent review is clear for 004f54e23cbef25eb739df2250ccfb7a66ffb976 against 5a895c5055396185f494a14bc364f9dd7de59080: no P0–P2 findings. Reviewer inspected the immutable production/test/doc diff and independently repeated authority, symlink/migration/retarget, subprocess, explicit/shared exclusion, CLI/MCP and integrated cancellation race controls five times, including both earlier reviewer regressions; all passed. Reviewer Go/subprocesses have ended. Required exact-head CI 38014534853 independently verified completed/success, workflow CI, push on the issue branch. Canonical PR head/base match; unresolved blocking discussions = 0. PR metadata refreshed to this SHA; issue input unchanged. Proceeding with SHA-guarded merge. Closure and release remain gated on resulting main CI.

likedislike
urandon
urandon成员
18 天前 评论:

!162 merged through an audited SHA-guarded operation. Resulting main commit: df3bb3396a1a0ad593f140ea517f5270fae7e009; reviewed head: 004f54e23cbef25eb739df2250ccfb7a66ffb976. Final review is clear and exact-head CI passed. Resulting main CI is now the remaining integration gate; issue remains open and no release tag has been created.

likedislike
urandon
urandon成员
18 天前 评论:

Completed: !162 merged at df3bb3396a1a0ad593f140ea517f5270fae7e009. Independent review clear on exact head 004f54e23cbef25eb739df2250ccfb7a66ffb976; head CI 38014534853 and resulting main CI 38014885130 completed/success, independently verified. Two consecutive simultaneous uncached-full/race gate pairs passed after integrating #171; reviewer authority/symlink/migration/subprocess and CLI/MCP controls passed ×5. Same-cache and explicit-shared exclusion remain tested; no schema, provider retry, or user-visible resource/action change. Epic #165 records dependency completion before this closure.

likedislike
urandonurandon成员
18 天前 关闭了 issue