On 04/06/2019 09:58, [email protected] wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> remm pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
> new a98f6d0 Fix rare ISE issue I see in TestAsyncFlush.NIO
> a98f6d0 is described below
>
> commit a98f6d02bb92ded2b3b0cb5438fb7c8adb8d85f9
> Author: remm <[email protected]>
> AuthorDate: Tue Jun 4 10:58:24 2019 +0200
>
> Fix rare ISE issue I see in TestAsyncFlush.NIO
>
> It looks possibly ok to me to wait on both allocation types with
> waitForNonBlocking (that's what the test runs into for me, both reach 0
> at the same time).
That is definitely not OK. It should never happen.
The sequence is:
- Request allocation from stream. In none available, wait until some is.
- Request allocation from connection. In none available, wait until some
is.
- Then write.
It should be impossible for a Stream to be waiting for an allocation
from the Stream and the Connection.
Mark
> Also use NONE instead of 0.
> Tests pass for me, so trying with CI.
> ---
> .../coyote/http2/WindowAllocationManager.java | 34
> +++++++++++-----------
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/java/org/apache/coyote/http2/WindowAllocationManager.java
> b/java/org/apache/coyote/http2/WindowAllocationManager.java
> index 7626bf3..6510312 100644
> --- a/java/org/apache/coyote/http2/WindowAllocationManager.java
> +++ b/java/org/apache/coyote/http2/WindowAllocationManager.java
> @@ -121,7 +121,7 @@ class WindowAllocationManager {
>
> private void waitFor(int waitTarget, long timeout) throws
> InterruptedException {
> synchronized (stream) {
> - if (waitingFor != 0) {
> + if (waitingFor != NONE) {
> throw new
> IllegalStateException(sm.getString("windowAllocationManager.waitFor.ise",
> stream.getConnectionId(), stream.getIdentifier()));
> }
> @@ -134,23 +134,21 @@ class WindowAllocationManager {
> stream.wait(timeout);
> }
>
> - waitingFor = 0;
> + waitingFor = NONE;
> }
> }
>
>
> private void waitForNonBlocking(int waitTarget) {
> synchronized (stream) {
> - if (waitingFor == 0) {
> + if (waitingFor == NONE) {
> waitingFor = waitTarget;
> } else if (waitingFor == waitTarget) {
> // NO-OP
> // Non-blocking post-processing may attempt to flush
> } else {
> - throw new
> IllegalStateException(sm.getString("windowAllocationManager.waitFor.ise",
> - stream.getConnectionId(), stream.getIdentifier()));
> + waitingFor |= waitTarget;
> }
> -
> }
> }
>
> @@ -162,7 +160,7 @@ class WindowAllocationManager {
> }
>
> synchronized (stream) {
> - if ((notifyTarget & waitingFor) > 0) {
> + if ((notifyTarget & waitingFor) > NONE) {
> if (stream.getCoyoteResponse().getWriteListener() == null) {
> // Blocking, so use notify to release StreamOutputBuffer
> if (log.isDebugEnabled()) {
> @@ -171,17 +169,19 @@ class WindowAllocationManager {
> }
> stream.notify();
> } else {
> - waitingFor = 0;
> - // Non-blocking so dispatch
> - if (log.isDebugEnabled()) {
> -
> log.debug(sm.getString("windowAllocationManager.dispatched",
> - stream.getConnectionId(),
> stream.getIdentifier()));
> + waitingFor &= ~notifyTarget;
> + if (waitingFor == NONE) {
> + // Non-blocking so dispatch
> + if (log.isDebugEnabled()) {
> +
> log.debug(sm.getString("windowAllocationManager.dispatched",
> + stream.getConnectionId(),
> stream.getIdentifier()));
> + }
> +
> stream.getCoyoteResponse().action(ActionCode.DISPATCH_WRITE, null);
> + // Need to explicitly execute dispatches on the
> StreamProcessor
> + // as this thread is being processed by an
> UpgradeProcessor
> + // which won't see this dispatch
> +
> stream.getCoyoteResponse().action(ActionCode.DISPATCH_EXECUTE, null);
> }
> -
> stream.getCoyoteResponse().action(ActionCode.DISPATCH_WRITE, null);
> - // Need to explicitly execute dispatches on the
> StreamProcessor
> - // as this thread is being processed by an
> UpgradeProcessor
> - // which won't see this dispatch
> -
> stream.getCoyoteResponse().action(ActionCode.DISPATCH_EXECUTE, null);
> }
> }
> }
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
