Fix NPE in PendingAddOp.maybeTimeout() when clientCtx is null after recycling

[eolivelli]

Motivation

monitorPendingAddOps() iterates over the pendingAddOps queue and calls
maybeTimeout() on each op. Concurrently, sendAddSuccessCallbacks() can
remove a completed op from the queue, call submitCallback() on it, and
ultimately trigger recyclePendAddOpObject() which sets clientCtx = null.

If the scheduler thread still holds a live iterator reference to the same op
and then calls maybeTimeout(), dereferencing clientCtx causes a
NullPointerException:

java.lang.NullPointerException
    at org.apache.bookkeeper.client.PendingAddOp.maybeTimeout(PendingAddOp.java:157)
    at org.apache.bookkeeper.client.LedgerHandle.monitorPendingAddOps(LedgerHandle.java:2063)

The race is only triggered when addEntryQuorumTimeoutNanos > 0 (i.e. the
quorum-timeout monitor is actually scheduled). It became reliably observable
with Netty 4.1.130, which changed Recycler thread-scheduling behavior
and narrowed the window enough to expose the pre-existing race.

Closes #4759.

Changes

PendingAddOp.java

• clientCtx → volatile ClientContext clientCtx
recyclePendAddOpObject() is synchronized but maybeTimeout() is not.
  Without volatile the JMM does not guarantee that the null write in
  recyclePendAddOpObject() is visible to the unsynchronized read in
  maybeTimeout(). Making the field volatile (consistent with the existing
  volatile long requestTimeNanos) closes the visibility gap.
• Null guard at the top of maybeTimeout()
  If clientCtx is null the op has already been recycled and its
  add-entry completed; there is nothing to time out, so the method
  returns false immediately.

PendingAddOpTest.java

Three new unit tests:

Test	What it verifies
testMaybeTimeoutReturnsFalseWhenClientCtxIsNull	The exact race: clientCtx = null must not NPE
testMaybeTimeoutReturnsFalseWhenWithinQuorumTimeout	Normal path — not yet timed out
testMaybeTimeoutReturnsTrueWhenQuorumTimeoutExpired	Normal path — timed out, true returned

[merlimat]

I don't think this would solve the race condition completely, since it could be set to null before the next read on line 163.

Better approach would be (even without the volatile) to assign to local variable and the the null there. Still wouldn't address whether it's safe to call timeoutQuorumWait() if it was already recycled.