Make outgoing pending count getters non-blocking - #1632

Claude finished @scottf's task in 43s —— View job

Review complete

• Fetch PR diff and description
• Examine changed files (NatsConnection, NatsConnectionWriter, tests)
• Check lock/thread-safety history for regressions
• Review new test correctness
• Post final review summary

Verified the core claim. WriterMessageQueue (MessageQueueBase) backs length/sizeInBytes with AtomicLong, and push/accumulate/filter/clear mutate them under editLock, not writerLock. writerLock only guards sendMessageBatch (the in-flight socket write loop) and flushBuffer, so it never provided any consistency for these getters — only added blocking. Same story for closeSocketLock: writer (NatsConnection.java:191) and normalOutgoing (NatsConnectionWriter.java:77) are each assigned exactly once, in their constructors, and never reassigned, so the removed null checks were dead code and the lock added no synchronization value, just reconnect-duration blocking.

Correctness/thread-safety: No issues. The getters now do a plain volatile-backed AtomicLong.get(), which is safe for concurrent reads against the atomic writers elsewhere. No new race is introduced — this removes locks that were never actually providing exclusion relevant to the read.

Test: testOutgoingPendingGettersDoNotBlockOnCloseSocketLock correctly reproduces the bug by holding closeSocketLock on the test thread and asserting the getters return promptly from another thread — this is a real regression test, not just a smoke test, and matches the PR's negative-control claim (fails on unmodified main).

API compatibility: Return values change from -1 (never actually reachable given final, once-assigned fields) to the real counts in that same dead branch — not an observable behavior change for any real caller.

Minor, non-blocking observation: the comment on NatsConnection.java:2672 says "the writer is created once in the constructor so the reference cannot change," which is accurate, but note writer is protected (not final) — harmless today since nothing reassigns it, just flagging that the safety relies on that invariant holding, same as before this PR.

No blocking issues found.

On the protected, non-final writer observation: the field was made protected in 62f6325 for custom Connection implementations, so making it final is a separate decision about that extension point and is not part of this PR. Nothing in src/main or src/test reassigns it after the constructor, including the three test subclasses of NatsConnection. The comment at NatsConnection.java:2672 overstated this as "the reference cannot change". It now states the actual invariant: assigned once in the constructor, nothing reassigns it, protected rather than final for custom implementations. That wording change goes up with the next push.