Issue #347 (Avoid sending request using a connection that is idle timing out).
Fixed by improving the guard with a timestamp, and checking that the time elapsed from the last timestamp is enough to prove it is a real idle timeout.
This commit is contained in:
parent
1cce6fd69f
commit
48c4e08b94
|
@ -23,6 +23,7 @@ import java.net.HttpCookie;
|
|||
import java.net.URI;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import java.util.concurrent.TimeoutException;
|
||||
import java.util.concurrent.atomic.AtomicInteger;
|
||||
|
||||
|
@ -45,8 +46,9 @@ public abstract class HttpConnection implements Connection
|
|||
private static final Logger LOG = Log.getLogger(HttpConnection.class);
|
||||
private static final HttpField CHUNKED_FIELD = new HttpField(HttpHeader.TRANSFER_ENCODING, HttpHeaderValue.CHUNKED);
|
||||
|
||||
private final AtomicInteger idleTimeoutState = new AtomicInteger();
|
||||
private final AtomicInteger idleTimeoutGuard = new AtomicInteger();
|
||||
private final HttpDestination destination;
|
||||
private long idleTimeoutStamp = System.nanoTime();
|
||||
|
||||
protected HttpConnection(HttpDestination destination)
|
||||
{
|
||||
|
@ -183,10 +185,10 @@ public abstract class HttpConnection implements Connection
|
|||
boolean send = false;
|
||||
while (true)
|
||||
{
|
||||
int current = idleTimeoutState.get();
|
||||
int current = idleTimeoutGuard.get();
|
||||
if (current < 0)
|
||||
break;
|
||||
if (idleTimeoutState.compareAndSet(current, current + 1))
|
||||
if (idleTimeoutGuard.compareAndSet(current, current + 1))
|
||||
{
|
||||
send = true;
|
||||
break;
|
||||
|
@ -207,7 +209,8 @@ public abstract class HttpConnection implements Connection
|
|||
channel.release();
|
||||
result = new SendFailure(new HttpRequestException("Could not associate request to connection", request), false);
|
||||
}
|
||||
idleTimeoutState.decrementAndGet();
|
||||
idleTimeoutStamp = System.nanoTime();
|
||||
idleTimeoutGuard.decrementAndGet();
|
||||
return result;
|
||||
}
|
||||
else
|
||||
|
@ -216,11 +219,22 @@ public abstract class HttpConnection implements Connection
|
|||
}
|
||||
}
|
||||
|
||||
public boolean onIdleTimeout()
|
||||
public boolean onIdleTimeout(long idleTimeout)
|
||||
{
|
||||
if (LOG.isDebugEnabled())
|
||||
LOG.debug("Idle timeout state {} - {}", idleTimeoutState, this);
|
||||
return idleTimeoutState.compareAndSet(0, -1);
|
||||
if (idleTimeoutGuard.compareAndSet(0, -1))
|
||||
{
|
||||
long elapsed = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - idleTimeoutStamp);
|
||||
boolean idle = elapsed > idleTimeout / 2;
|
||||
if (LOG.isDebugEnabled())
|
||||
LOG.debug("Idle timeout {}/{}ms - {}", elapsed, idleTimeout, this);
|
||||
return idle;
|
||||
}
|
||||
else
|
||||
{
|
||||
if (LOG.isDebugEnabled())
|
||||
LOG.debug("Idle timeout skipped - {}", this);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
@ -98,9 +98,10 @@ public class HttpConnectionOverHTTP extends AbstractConnection implements Connec
|
|||
@Override
|
||||
public boolean onIdleExpired()
|
||||
{
|
||||
boolean close = delegate.onIdleTimeout();
|
||||
long idleTimeout = getEndPoint().getIdleTimeout();
|
||||
boolean close = delegate.onIdleTimeout(idleTimeout);
|
||||
if (close)
|
||||
close(new TimeoutException("Idle timeout " + getEndPoint().getIdleTimeout() + "ms"));
|
||||
close(new TimeoutException("Idle timeout " + idleTimeout + "ms"));
|
||||
return false;
|
||||
}
|
||||
|
||||
|
|
|
@ -168,7 +168,9 @@ public class HttpReceiverOverHTTPTest
|
|||
HttpExchange exchange = newExchange();
|
||||
FutureResponseListener listener = (FutureResponseListener)exchange.getResponseListeners().get(0);
|
||||
connection.getHttpChannel().receive();
|
||||
// Simulate an idle timeout
|
||||
// ByteArrayEndPoint has an idle timeout of 0 by default,
|
||||
// so to simulate an idle timeout is enough to wait a bit.
|
||||
Thread.sleep(100);
|
||||
connection.onIdleExpired();
|
||||
|
||||
try
|
||||
|
|
|
@ -193,11 +193,12 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements Connec
|
|||
@Override
|
||||
public boolean onIdleExpired()
|
||||
{
|
||||
boolean close = delegate.onIdleTimeout();
|
||||
long idleTimeout = getEndPoint().getIdleTimeout();
|
||||
boolean close = delegate.onIdleTimeout(idleTimeout);
|
||||
if (multiplexed)
|
||||
close &= isFillInterested();
|
||||
if (close)
|
||||
close(new TimeoutException("Idle timeout " + getEndPoint().getIdleTimeout() + "ms"));
|
||||
close(new TimeoutException("Idle timeout " + idleTimeout + "ms"));
|
||||
return false;
|
||||
}
|
||||
|
||||
|
|
|
@ -30,6 +30,7 @@ import org.eclipse.jetty.client.HttpDestination;
|
|||
import org.eclipse.jetty.client.Origin;
|
||||
import org.eclipse.jetty.client.api.Connection;
|
||||
import org.eclipse.jetty.http.HttpScheme;
|
||||
import org.eclipse.jetty.http2.HTTP2Session;
|
||||
import org.eclipse.jetty.http2.api.Session;
|
||||
import org.eclipse.jetty.http2.client.HTTP2Client;
|
||||
import org.eclipse.jetty.http2.client.HTTP2ClientConnectionFactory;
|
||||
|
@ -187,7 +188,7 @@ public class HttpClientTransportOverHTTP2 extends ContainerLifeCycle implements
|
|||
@Override
|
||||
public boolean onIdleTimeout(Session session)
|
||||
{
|
||||
return connection.onIdleTimeout();
|
||||
return connection.onIdleTimeout(((HTTP2Session)session).getEndPoint().getIdleTimeout());
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
Loading…
Reference in New Issue