https://bz.apache.org/bugzilla/show_bug.cgi?id=70290
Ben Moss <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- OS| |All --- Comment #1 from Ben Moss <[email protected]> --- A second path to the same leak, not covered by PR #1125 (tested at 76ec87a). If a worker thread's write fails after CoyoteAdapter.service() has checked for errors but before asyncPostProcess() runs, asyncError() moves the state from STARTING to MUST_ERROR. asyncPostProcess() has no MUST_ERROR branch: java.lang.IllegalStateException: Calling [asyncPostProcess()] is not valid for a request with Async state [MUST_ERROR] at org.apache.coyote.AsyncStateMachine.asyncPostProcess(AsyncStateMachine.java) at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java) The request is then recycled forcibly. Neither onError() nor onComplete() is called, and inProgressAsyncCount is not decremented. The application does not need to call dispatch() or complete(): an async servlet whose worker is writing when the client sends a RST is enough. asyncError() was called off a container thread, so it already queued an ERROR dispatch. That dispatch is blocked on the socket lock and finds nothing to process once the request is recycled. Repro: a variant of TestAsyncResetInProgressCount with no dispatch(), plus a test-only hook just before asyncPostProcess() to force the timing. It failed 20 of 20 rounds on PR #1125, with the count rising by one per round. Proposed fix: handle MUST_ERROR like STARTING, so the queued ERROR dispatch runs the normal onError() and complete() handling: + } else if (state == AsyncState.MUST_ERROR) { + // asyncError() on a non-container thread has already triggered an + // ERROR dispatch. Keep the processor so that dispatch can call + // onError() and complete the async processing as usual. + updateState(AsyncState.ERROR); + return SocketState.LONG; On top of PR #1125, the repro then passes 20 of 20 rounds (onError() and onComplete() each called once, count balanced). TestAsync*, catalina/connector, coyote and coyote/http11 all pass (829 tests). I chose this over calling onComplete() directly, as the MUST_DISPATCH fix does, because that would skip onError(). -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
