Skip to content

[PR] ci-fixes convergence: port the four remaining master-red lanes (win32 db path, teardown handles, #5155 settle, #4788 timer host) - #5262

Merged
ranxianglei merged 1 commit into
ranxianglei:ci-fixesfrom
Louie0806:2026-10-30_ci-fixes-two-lanes
Oct 30, 2026
Merged

ranxianglei merged 1 commit into
ranxianglei:ci-fixesfrom
Louie0806:2026-10-30_ci-fixes-two-lanes

Conversation

@Louie0806

@Louie0806 Louie0806 commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

(ai: deepseek/deepseek-v41-flash)

ci-fixes still leaves 4 shards red — and all four fixes already exist on #5178

At 67f8e894, the convergence branch turns some of master's red lanes green, but four remain. Every one of them is a missing delta: the fix exists on PR #5178 and was simply never ported here. This PR ports the missing hunks so the convergence branch actually converges.

shard failing suite root cause
test (windows-latest, 24, 3) tests/issue5103-startup-recovery-visibility.test.ts win32 separator in the sidecar db path
test (windows-latest, 22, 4) tests/issue4739-bounded-session-load.test.ts (both full-mode cases) open sqlite handle blocks unlink
test (windows-latest, 24, 5) tests/issue4788-resident-minimal.test.ts (d) same open-handle family
test (ubuntu-latest, 24, 3) tests/plugin-fork.test.ts #5140 cold-restored raw tail fixed setTimeout(20) budget missed a debounced flush
test (ubuntu-latest, 22, 3) tests/issue4788-resident-minimal.test.ts (a)–(d) unref'd debounce timer never drains afterNextFlush

1. #5103 — a mixed separator, not a missing warning

UnifiedShadow builds its db path as ${dir}/index.db, so on Windows the open-failure warning reads C:\...\sessions/index.db. The assertion needle is path.join(dir, "index.db") — ...\index.db on win32 — so the mixed form never matches, on every Windows shard. path is already imported at src/storage/shadow.ts:30, and the db is opened through that same join, so POSIX output is byte-identical. I proved the mismatch mechanically:

old message = C:\Users\...\bili-5103-shadow-abcd/index.db
test needle = C:\Users\...\bili-5103-shadow-abcd\index.db
OLD passes? false     NEW passes? true

2. #4739 / #4788 teardown — the sidecar is still open

These suites arm a real SessionStorage under BILI_STORAGE_UNIFIED=full, so index.db is open at teardown. cancelAll() (src/persist.ts:3230) clears timers and the build map but never calls this.shadow?.close(); Windows refuses to unlink a directory holding an open file. The shared rmrf retries transient EBUSY/EPERM for ~9.5s, so a failure here means the handle genuinely never closes — the observed duration_ms: 11877 is exactly that budget expiring.

closeQuietly() (src/persist.ts:3193) is cancelAll() + shadow.close(). Only the short-lived-store teardowns need it; the UnifiedStore.open(...) cases already call store.close().

This hunk has a portable negative control — no Windows needed. /proc/self/fd shows the leak directly:

cancelAll:    before=["index.db","index.db-shm","index.db-wal"] after=["index.db","index.db-shm","index.db-wal"] unlink=ok
closeQuietly: before=["index.db","index.db-shm","index.db-wal"] after=[]                                    unlink=ok

Three open handles survive cancelAll(), zero survive closeQuietly(). (Linux unlink succeeds on open files, which is precisely why a Linux-only test cannot fail without this probe.)

3. #5155 — a millisecond budget is a deadline

Both cold-rebuild cases waited for the debounced durable flush with a fixed setTimeout(20). A magic budget is a deadline a loaded runner can miss, and the miss is an honest 409 !== 200 from a one-turn-old table. Await the settle itself — afterNextFlush("conv-ptrs") resolves once the armed save lands, with a landing proof.

4. #4788 — the unref'd timer deadlocks the verdict

afterNextFlush parks a resolver in flushWaiters; the only thing that drains it is the debounce timer — armed with timer.unref?.() at src/persist.ts:2769. Once that timer is the last pending handle, the loop drains, the promise never settles, and node reports Promise resolution is still pending but the event loop has already resolved. Keep the timer referenced exactly while a waiter exists (arm unref'd, ref() on waiter registration). This is the one lane with a deterministic local repro:

issue4788 on 67f8e894:                      pass 0 / fail 4
  + timer-ref hunk:                       pass 4 / fail 0
  revert only the timer-ref hunk:         pass 0 / fail 4

That is a real double-direction control, not an assertion loosened until green.

Verification

npx tsc --noEmit                          exit 0
issue4788-resident-minimal                4/4
inc-canonical-pointers                    3/3
issue4739-bounded-session-load            4/4
issue5103-startup-recovery-visibility     5/5

Honest scope: the two teardown hunks have no local negative control — reverting closeQuietly() keeps Linux green because EBUSY/EPERM only fire where an open handle blocks unlink. For those, the /proc/self/fd probe above is the evidence, and the CI verdict is the Windows shards.

Close this if the sentinel already folded these in; the value is the four deltas plus the line numbers, not the branch.

Coordination: #4599, #5178, #5208.

…nto ci-fixes

ci-fixes at 67f8e89 still leaves four Windows shards red, and all four
deltas already exist on PR ranxianglei#5178. This ports the missing hunks so the
convergence branch actually converges.

1. `test (windows-latest, 2x, 3)` — ranxianglei#5103. `UnifiedShadow` builds its db
   path as `${dir}/index.db`, so the open-failure warning reads
   `C:\...\sessions/index.db`. `issue5103` asserts the warning contains
   `path.join(dir, "index.db")` — `...\index.db` on win32 — so a mixed
   separator never matches. `path` is already imported at :30 and the db is
   opened through the same join, so POSIX output is byte-identical.

2. `test (windows-latest, 22, 4)` + the 4788 leg — ranxianglei#4739/ranxianglei#4788 teardown.
   The suites arm a real `SessionStorage` under
   `BILI_STORAGE_UNIFIED=full`, so the sidecar is still open at teardown.
   `cancelAll()` clears timers and the build map but never closes the
   shadow; Windows refuses to unlink a directory holding an open file
   (`EBUSY`/`EPERM` on `index.db`). Use `closeQuietly()` — cancel, then
   close — in the short-lived-store teardown.

3. `test (ubuntu-latest, 24, 3)` + `(windows-latest, 22, 2)` — ranxianglei#5155.
   Both cold-rebuild cases waited for the debounced durable flush with a
   fixed `setTimeout(20)`. A millisecond budget is a deadline a loaded
   runner can miss, and the miss is an honest `409 !== 200` from a
   one-turn-stale table. Await the settle itself: `afterNextFlush("conv-ptrs")` resolves once the armed save lands.

4. `test (ubuntu-latest, 22, 3)` — ranxianglei#4788 verdict deadlock.
   `afterNextFlush` parks a resolver in `flushWaiters`, and the only thing
   that drains it is the debounce timer — which is armed `unref?.()`.
   Once that timer is the last pending handle the loop drains, the promise
   never settles, and node reports
   `Promise resolution is still pending but the event loop has already
   resolved`. Keep the timer referenced exactly while a waiter exists
   (`ref()` on waiter registration; the host-upkeep default stays unref'd).

Both view hunks and the two test-teardown hunks are byte-identical to the
ones already on PR ranxianglei#5178, so this converges the branches instead of
inventing a third variant.

Verified locally: `npx tsc --noEmit` clean; issue4788 0/4 -> 4/4;
issue4739 4/4; issue5103 5/5; inc-canonical-pointers 3/3.

The `cancelAll` -> `closeQuietly` hunk has no local negative control:
reverting it keeps Linux green because `EBUSY`/`EPERM` only fire where an
open handle blocks unlink. That control can only run on a Windows lane —
stated rather than papered over.

The ranxianglei#4788 arm's own reader is confirmed instead: `cancelAll()` leaves
`index.db`, `index.db-shm`, `index.db-wal` in `/proc/self/fd` while
`closeQuietly()` leaves none (probed directly).
@Louie0806
Louie0806 force-pushed the 2026-10-30_ci-fixes-two-lanes branch from 6cb939c to 421026d Compare October 30, 2026 18:22
@Louie0806 Louie0806 changed the title [PR] ci-fixes convergence: the two remaining master-red lanes (win32 db path + #4739 teardown handle) [PR] ci-fixes convergence: port the four remaining master-red lanes (win32 db path, teardown handles, #5155 settle, #4788 timer host) Oct 30, 2026
@ranxianglei
ranxianglei merged commit f521b0f into ranxianglei:ci-fixes Oct 30, 2026
8 checks passed
ranxianglei pushed a commit that referenced this pull request Oct 30, 2026
…ts ref-tracking fix (#4599)

PR #5262 fixes #4788 at the source: scheduleSave now leaves the debounce
timer refd while flush waiters exist (and afterNextFlush refs a pending
unrefd timer when a waiter registers), so the settle is deterministic for
EVERY caller on every host. The 25 ms keep-alive interval added in
a3019b0 only patched this one test file and is redundant now; removing it
leaves one mechanism instead of two. The comment stays: without it a future
reader re-adds the interval or deletes the persist.ts tracking as dead weight.
ranxianglei pushed a commit that referenced this pull request Oct 30, 2026
…x reds (#4599)

Brings in the sentinel-converged fixes for the two red lanes on head 44e1f86: #5103 win32 warning separator (shadow.ts path.join) and #4739 EBUSY teardown (closeQuietly before rmrf), plus the #4788 persist.ts ref-tracking root fix (PR #5262) and inc-canonical-pointers settle hardening. Conflict in tests/plugin-fork.test.ts resolved by keeping this branch's two-mode (full+shadow) #5140 pin loop — strict superset of the ci-fixes single-full pin. No new content of this PR touched.
ranxianglei pushed a commit to adlternative/billion-context that referenced this pull request Oct 30, 2026
…port (issue5103 win32 db-path, issue4739 closeQuietly, inc-canonical-pointers) + ranxianglei#4788 timer host (ranxianglei#4599)
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.

2 participants