From 8cfa67125357da10158ea9b91da3d09b1a749f48 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 9 Nov 2011 09:16:04 +1100 Subject: [PATCH] only handle early EOF if exchange is not done --- .../jetty/client/AbstractHttpConnection.java | 11 ++++++---- .../jetty/client/AsyncHttpConnection.java | 4 ++-- .../jetty/client/BlockingHttpConnection.java | 4 ++-- .../eclipse/jetty/client/HttpDestination.java | 17 +++++++-------- .../eclipse/jetty/client/HttpExchange.java | 21 ++++++++++++------- .../AbstractHttpExchangeCancelTest.java | 9 ++++---- .../NonBlockingHttpExchangeCancelTest.java | 6 ++++++ 7 files changed, 44 insertions(+), 28 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractHttpConnection.java b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractHttpConnection.java index 57a56325fc1..868dbd4ba58 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractHttpConnection.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractHttpConnection.java @@ -346,8 +346,11 @@ public abstract class AbstractHttpConnection extends AbstractConnection implemen HttpExchange exchange = _exchange; if (exchange!=null) { - exchange.setStatus(HttpExchange.STATUS_EXCEPTED); - exchange.getEventListener().onException(new EofException("early EOF")); + if (!exchange.isDone()) + { + if (exchange.setStatus(HttpExchange.STATUS_EXCEPTED)) + exchange.getEventListener().onException(new EofException("early EOF")); + } } } @@ -389,8 +392,8 @@ public abstract class AbstractHttpConnection extends AbstractConnection implemen default: String exch= exchange.toString(); String reason = _endp.isOpen()?(_endp.isInputShutdown()?"half closed: ":"local close: "):"closed: "; - exchange.setStatus(HttpExchange.STATUS_EXCEPTED); - exchange.getEventListener().onException(new EofException(reason+exch)); + if (exchange.setStatus(HttpExchange.STATUS_EXCEPTED)) + exchange.getEventListener().onException(new EofException(reason+exch)); } } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/AsyncHttpConnection.java b/jetty-client/src/main/java/org/eclipse/jetty/client/AsyncHttpConnection.java index 143056b970c..44745204851 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/AsyncHttpConnection.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/AsyncHttpConnection.java @@ -161,8 +161,8 @@ public class AsyncHttpConnection extends AbstractHttpConnection implements Async exchange.getStatus() != HttpExchange.STATUS_CANCELLED && !exchange.isDone()) { - exchange.setStatus(HttpExchange.STATUS_EXCEPTED); - exchange.getEventListener().onException(e); + if (exchange.setStatus(HttpExchange.STATUS_EXCEPTED)) + exchange.getEventListener().onException(e); } } else diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/BlockingHttpConnection.java b/jetty-client/src/main/java/org/eclipse/jetty/client/BlockingHttpConnection.java index 6eb6b34686c..c02e156911b 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/BlockingHttpConnection.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/BlockingHttpConnection.java @@ -162,8 +162,8 @@ public class BlockingHttpConnection extends AbstractHttpConnection exchange.getStatus() != HttpExchange.STATUS_CANCELLED && !exchange.isDone()) { - exchange.setStatus(HttpExchange.STATUS_EXCEPTED); - exchange.getEventListener().onException(e); + if(exchange.setStatus(HttpExchange.STATUS_EXCEPTED)) + exchange.getEventListener().onException(e); } } else diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java index 38d8bfbb49c..a0e40ef15f1 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java @@ -294,8 +294,8 @@ public class HttpDestination implements Dumpable else if (_queue.size() > 0) { HttpExchange ex = _queue.remove(0); - ex.setStatus(HttpExchange.STATUS_EXCEPTED); - ex.getEventListener().onConnectionFailed(throwable); + if (ex.setStatus(HttpExchange.STATUS_EXCEPTED)) + ex.getEventListener().onConnectionFailed(throwable); // Since an existing connection had failed, we need to create a // connection if the queue is not empty and client is running. @@ -328,8 +328,8 @@ public class HttpDestination implements Dumpable if (_queue.size() > 0) { HttpExchange ex = _queue.remove(0); - ex.setStatus(HttpExchange.STATUS_EXCEPTED); - ex.getEventListener().onException(throwable); + if(ex.setStatus(HttpExchange.STATUS_EXCEPTED)) + ex.getEventListener().onException(throwable); } } } @@ -428,7 +428,6 @@ public class HttpDestination implements Dumpable synchronized (this) { _connections.remove(connection); - System.err.println("Q "+_queue); if (!_queue.isEmpty()) startConnection = true; } @@ -717,16 +716,16 @@ public class HttpDestination implements Dumpable protected void onException(Throwable x) { _queue.remove(exchange); - exchange.setStatus(STATUS_EXCEPTED); - exchange.getEventListener().onException(x); + if (exchange.setStatus(STATUS_EXCEPTED)) + exchange.getEventListener().onException(x); } @Override protected void onExpire() { _queue.remove(exchange); - exchange.setStatus(STATUS_EXPIRED); - exchange.getEventListener().onExpire(); + if (exchange.setStatus(STATUS_EXPIRED)) + exchange.getEventListener().onExpire(); } } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpExchange.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpExchange.java index 256387f3c3f..2440707ca68 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpExchange.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpExchange.java @@ -181,12 +181,18 @@ public class HttpExchange } } - void setStatus(int newStatus) + /* ------------------------------------------------------------ */ + /** + * @param newStatus + * @return True if the status was actually set. + */ + boolean setStatus(int newStatus) { + boolean set = false; try { int oldStatus = _status.get(); - boolean set = false; + boolean ignored = false; if (oldStatus != newStatus) { long now = System.currentTimeMillis(); @@ -313,7 +319,7 @@ public class HttpExchange case STATUS_CANCELLING: case STATUS_EXPIRED: // Don't change the status, it's too late - set = true; + ignored = true; break; } break; @@ -327,7 +333,7 @@ public class HttpExchange break; default: // Ignore other statuses, we're cancelling - set = true; + ignored = true; break; } break; @@ -341,12 +347,12 @@ public class HttpExchange break; case STATUS_COMPLETED: - set = true; + ignored = true; done(); break; default: - set = true; + ignored = true; break; } break; @@ -355,7 +361,7 @@ public class HttpExchange throw new AssertionError(oldStatus + " => " + newStatus); } - if (!set) + if (!set && !ignored) throw new IllegalStateException(toState(oldStatus) + " => " + toState(newStatus)); LOG.debug("setStatus {} {}",newStatus,this); } @@ -363,6 +369,7 @@ public class HttpExchange { LOG.warn(x); } + return set; } private boolean setStatusExpired(int newStatus, int oldStatus) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/AbstractHttpExchangeCancelTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/AbstractHttpExchangeCancelTest.java index 26567499f0a..6a96196d84a 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/AbstractHttpExchangeCancelTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/AbstractHttpExchangeCancelTest.java @@ -76,13 +76,13 @@ public abstract class AbstractHttpExchangeCancelTest TestHttpExchange exchange = new TestHttpExchange() { @Override - void setStatus(int status) + boolean setStatus(int status) { // Cancel before setting the new status if (getStatus() == HttpExchange.STATUS_START && status == STATUS_WAITING_FOR_CONNECTION) cancel(); - super.setStatus(status); + return super.setStatus(status); } }; exchange.setAddress(newAddress()); @@ -113,14 +113,15 @@ public abstract class AbstractHttpExchangeCancelTest TestHttpExchange exchange = new TestHttpExchange() { @Override - void setStatus(int status) + boolean setStatus(int status) { // Cancel after setting the new status int oldStatus = getStatus(); - super.setStatus(status); + boolean set = super.setStatus(status); if (oldStatus == STATUS_START && getStatus() == HttpExchange.STATUS_WAITING_FOR_CONNECTION) cancel(); + return set; } }; exchange.setAddress(newAddress()); diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/NonBlockingHttpExchangeCancelTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/NonBlockingHttpExchangeCancelTest.java index 7fdde5b2a2c..9950d6710fb 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/NonBlockingHttpExchangeCancelTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/NonBlockingHttpExchangeCancelTest.java @@ -51,4 +51,10 @@ public class NonBlockingHttpExchangeCancelTest extends AbstractHttpExchangeCance { return httpClient; } + + /* ------------------------------------------------------------ */ + public void testHttpExchangeCancelOnRequestComplete() throws Exception + { + super.testHttpExchangeCancelOnRequestComplete(); + } }