PDGGK opened a new pull request, #19:
URL: https://github.com/apache/iotdb-client-nodejs/pull/19

   This is the pool-lifecycle PR (findings 1-5) from the dev@ discussion 
"[DISCUSS] Hardening the iotdb-client-nodejs session pool / connection 
lifecycle", where the findings and this two-PR split were reviewed.
   
   ### Problems
   Five related lifecycle bugs in `BaseSessionPool` where session bookkeeping 
(`pool` / `idleSessions` / `activeSessions` / `waitQueue`) is updated 
inconsistently across `await` points and on the timeout/cleanup paths.
   
   **1. Timed-out waiter leaks a session → pool starvation.** The wait-timeout 
handler removed the stale waiter with `waitQueue.indexOf(resolve)`, but the 
queue stores a wrapper closure, not `resolve`, so `indexOf` is always `-1` and 
the timed-out waiter is never removed. A later `releaseSession()` then shifts 
that dead waiter, marks the session active, and resolves an already-rejected 
promise — leaking the session and eventually starving the pool.
   
   **2. Create-branch evicts the wrong idle session under interleaving.** After 
`await createSession()` (which pushes the new session to the back of idle), the 
branch did a blind `idleSessions.shift()` (front); under interleaving (a 
session released into idle during the await) it evicted a different session and 
left the new one tracked as both idle and active.
   
   **3. Idle-reuse branch never marks the session in use.** The idle-reuse path 
adds the session to `activeSessions` but omits `inUse = true` (the new-session 
and waiter branches both set it), so `syncDatabaseContextToPool` (which filters 
`!inUse`) treats an actively-in-use session as idle and can run `USE <db>` on 
it concurrently with the caller's in-flight request.
   
   **4. `cleanupIdleSessions()` can shrink the pool below `minPoolSize`.** The 
candidate guard checks `pool.length > minSize` against the constant pre-cleanup 
size, so it can queue every idle session and collapse the pool to 0.
   
   **5. `cleanupIdleSessions()` can hand out a session being closed.** It 
awaited `session.close()` before removing the session from pool/idle; since 
`isOpen()` stays true until the close resolves, a concurrent `getSession()` 
could shift and hand out a session that is being destroyed.
   
   ### Fixes
   - Wait-branch wrapper is a named `waiter` returning boolean, guarded by a 
`settled` flag; the timeout removes that exact `waiter`; `releaseSession()` 
loops over waiters skipping already-settled ones and falls back to idle.
   - Create-branch removes the *specific* new session (`indexOf` + `remove`) 
not `shift()`.
   - Idle-reuse branch sets `inUse = true`.
   - Cleanup guard uses the projected size (`pool.length - 
sessionsToRemove.length > minSize`).
   - Cleanup splices the session out of pool/idle *before* awaiting `close()`.
   
   ### Tests
   `tests/unit/BaseSessionPoolLifecycle.test.ts` (new) reproduces all five with 
a fake-session subclass; each fails on current `develop` and passes with this 
change. Full `tests/unit` green (159 tests).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to