Skip to content

Commit

Permalink
Fix #6227 Async timeout dispatch race (#6228)
Browse files Browse the repository at this point in the history
Fix #6227 Async timeout dispatch race
Only allow the thread calling onTimeout to call dispatch and complete once timeout has expired.
Signed-off-by: Greg Wilkins <gregw@webtide.com>
  • Loading branch information
gregw committed May 4, 2021
1 parent 2f19c67 commit ad6d23f
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 20 deletions.
Expand Up @@ -155,6 +155,7 @@ public enum Action
private boolean _asyncWritePossible;
private long _timeoutMs = DEFAULT_TIMEOUT;
private AsyncContextEvent _event;
private Thread _onTimeoutThread;

protected HttpChannelState(HttpChannel channel)
{
Expand Down Expand Up @@ -582,7 +583,10 @@ public void dispatch(ServletContext context, String path)
switch (_requestState)
{
case ASYNC:
break;
case EXPIRING:
if (Thread.currentThread() != _onTimeoutThread)
throw new IllegalStateException(this.getStatusStringLocked());
break;
default:
throw new IllegalStateException(this.getStatusStringLocked());
Expand Down Expand Up @@ -646,36 +650,47 @@ protected void onTimeout()
throw new IllegalStateException(toStringLocked());
event = _event;
listeners = _asyncListeners;
_onTimeoutThread = Thread.currentThread();
}

if (listeners != null)
try
{
Runnable task = new Runnable()
if (listeners != null)
{
@Override
public void run()
Runnable task = new Runnable()
{
for (AsyncListener listener : listeners)
@Override
public void run()
{
try
{
listener.onTimeout(event);
}
catch (Throwable x)
for (AsyncListener listener : listeners)
{
LOG.warn("{} while invoking onTimeout listener {}", x, listener, x);
try
{
listener.onTimeout(event);
}
catch (Throwable x)
{
LOG.warn("{} while invoking onTimeout listener {}", x, listener, x);
}
}
}
}

@Override
public String toString()
{
return "onTimeout";
}
};
@Override
public String toString()
{
return "onTimeout";
}
};

runInContext(event, task);
runInContext(event, task);
}
}
finally
{
synchronized (this)
{
_onTimeoutThread = null;
}
}
}

Expand All @@ -692,6 +707,11 @@ public void complete()
switch (_requestState)
{
case EXPIRING:
if (Thread.currentThread() != _onTimeoutThread)
throw new IllegalStateException(this.getStatusStringLocked());
_requestState = _sendError ? RequestState.BLOCKING : RequestState.COMPLETE;
break;

case ASYNC:
_requestState = _sendError ? RequestState.BLOCKING : RequestState.COMPLETE;
break;
Expand Down
Expand Up @@ -235,7 +235,8 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
}
catch (IllegalStateException x)
{
LOG.warn(x);
if (LOG.isDebugEnabled())
LOG.debug(x);
continue;
}
}
Expand Down

0 comments on commit ad6d23f

Please sign in to comment.