UrlProtocol · TcpServer · request for second opinion

A server that refuses capacity nothing is using

TcpServer reports its concurrent-handler limit to callers that are within that limit. The cause is settled and reproducible. What I want challenged is the fix: closing it completely costs a platform-thread bound that was deliberately established one commit earlier, and I believe that trade is unavoidable rather than a failure of imagination. That belief is the thing to attack.

Repo: CodexCoder21Organization/UrlProtocol Base: e804219c Defect arrived in: PR #493

What is observably wrong

A caller that completes one request and immediately issues the next receives: 

RpcError(code=CONCURRENT_HANDLER_LIMIT,
  message=TcpServer on port 41571 is at its concurrent request-handler limit of 2;
          request recovered-request for method 'status' was not started)

At the moment of that refusal, no handler is running. The statement is false, and it is user-visible: the caller is told to back off by a server that is idle.

This surfaced as a red kotlin.build (remote) check — testTcpServerRejectsRequestsAtConcurrentHandlerLimitAndRecovers, 1 failure in 1372 tests, retries exhausted. It passes on an unloaded local machine and fails on the loaded CI droplet, which is the signature of a timing window rather than a broken assertion.

Mechanism

Admission was never counted. It was inferred from a thread pool: 

ThreadPoolExecutor(
    0,                          // corePoolSize
    concurrentHandlerLimit,     // maximumPoolSize
    REQUEST_THREAD_KEEP_ALIVE_SECONDS, TimeUnit.SECONDS,
    SynchronousQueue(),
    threadFactory,
    ThreadPoolExecutor.AbortPolicy(),
)

A SynchronousQueue has no capacity of its own. execute() succeeds only if it can hand the task to a worker currently parked in poll(), or if it may create a new thread. Once poolSize has reached maximumPoolSize, the second option is gone — so capacity means “a worker is parked right now”, not “a handler slot is free”.

Those two things diverge for as long as it takes the scheduler to run a worker that has just finished. Ordered by happens-before:

1
reader threadReads request i, calls execute(). A worker takes it.
2
workerRuns the handler and writes the response to the socket.
3
worker — the windowReturns from the task and begins unwinding toward poll(). It is not yet parked, and poolSize == maximumPoolSize.
4
clientHas already read the response from step 2. Sends request i+1.
5
reader threadexecute() finds no parked worker and may not create one. RejectedExecutionException → CONCURRENT_HANDLER_LIMIT.
6
workerFinally parks. Capacity “returns”, having never actually been used.

Steps 3–5 are the defect. CPU contention widens the gap between 3 and 6, which is exactly why it reproduces on a loaded CI droplet and hides on an idle laptop.

Reproducer

The assertion is designed to be machine-independent. N client lanes each issue requests strictly sequentially, waiting for each response before sending the next, and there are exactly concurrentHandlerLimit lanes. At most concurrentHandlerLimit requests can be in flight at any instant, so the server is never genuinely saturated and no response may carry CONCURRENT_HANDLER_LIMIT. One such response is a failure.

The iteration count samples the window; it is not load, and it is not a threshold. Nothing is tuned to a machine — a faster host changes how often step 4 lands inside steps 3–6, not whether the assertion is correct.

Why the failure distribution confirms the mechanism

On unfixed main, every lane failed at request index 1 — the very first request after its first response. Request 0 creates the worker; the worker finishes and writes; request 1 arrives before it has re-parked. That is step 3 exactly, and it is the reason I stopped looking for a load explanation.

Two variants, measured

Both replace the inferred capacity with an explicit Semaphore(concurrentHandlerLimit) acquired before dispatch, and let the pool grow unbounded so it can never reject an admitted request. They differ only in when the permit is released. Same reproducer, same host, 1600 requests.

Variant Permit released Spurious refusals PR #493 test
Unfixed main n/a — capacity inferred from the pool 23 / 1600 fails on CI
A — semaphore after the response is written 1 / 1600 passes
B — semaphore after the handler returns, before the write 0 / 1600 passes

Variant A removes the scheduler from the accounting and fixes the reported CI failure, but leaves a narrower instance of the same class: the client reads the response before the handler thread executes release(). Variant B releases after the handler returns and before any byte can reach the caller, so a caller holding a response is guaranteed the slot that served it is already free.

The trade, and why I think it is inherent

This is the part I most want a second opinion on, because the argument is short enough to be suspicious.

The argument

For the server never to refuse a caller that respects the limit, nothing that gates admission may still be held while the response is being written — otherwise a caller holding its response can still be refused, which is the bug. But the thread is held through the write, by construction. Therefore threads cannot be the gate.

If that holds, variant B's cost is not an oversight but the price: with the permit released before the write, a peer that pipelines requests while not reading its responses is no longer refused, and its handlers accumulate threads instead — one writing, the rest blocked on the connection's output monitor. Previously the held slot supplied that backpressure. That backpressure is what PR #493 established one commit earlier, specifically to bound platform threads.

My reading is that bounding a stalled peer belongs at the socket layer — per-connection in-flight response limits, or write deadlines — not in handler-slot accounting, and that refusing unrelated well-behaved callers is the wrong way to get it. But I hold that loosely, and a redesign that bounds writes properly would dominate both variants.

Evidence that partly cuts against my own concern

Under variant B all 26 existing TCP tests pass, including testTcpPushIsBoundedWhenClientStopsReading — the repository's own test for the stalled-reader case I am worried about. So the concern is not currently demonstrated by any test in the suite. Either the existing coverage does not reach the accumulation path, or the path is bounded by something I have not identified. I could not tell which, and that uncertainty is why this document exists rather than just a merged PR.

What I would like attacked

  1. Is the inherency argument actually sound, or is there a formulation where admission is gated by something released before the write while threads stay bounded — a bounded writer stage, non-blocking writes, or per-connection in-flight caps?
  2. Is “a caller within the limit is never refused” the right contract at all? The alternative is that a slot is occupied until its response is fully written, which makes variant A correct and makes my stricter reproducer wrong rather than the server.
  3. Variant B changes behaviour under a hostile or stalled peer, in a subsystem hardened for exactly that one commit earlier. Is shipping it without the socket-layer bound in place too risky to do in one step?
  4. Is 1600 requests on one host adequate evidence that B is 0 rather than merely rarer than A? I did not run a power analysis, and A's residual was 1 in 1600.