EndPoint onIdleExpired should not close on its own
+ From discussion with Simone, the dispatched fillInterest.onFail()
needs to occur, so that the AbstractConnection.onReadTimeout() can
handle the close conditions needed by SPDY and WebSocket.
The EndPoint close within onIdleExpired is a race condition with
this onReadTimeout desired behavior.
diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java b/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java
index 6517dbd..bfcb1aa 100644
--- a/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java
+++ b/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java
@@ -142,12 +142,9 @@
@Override
protected void onIdleExpired(TimeoutException timeout)
{
- boolean output_shutdown=isOutputShutdown();
- boolean input_shutdown=isInputShutdown();
+ // Note: Rely on fillInterest to notify onReadTimeout to close connection.
_fillInterest.onFail(timeout);
_writeFlusher.onFail(timeout);
- if (isOpen() && output_shutdown || input_shutdown)
- close();
}
@Override
diff --git a/jetty-io/src/test/java/org/eclipse/jetty/io/ByteArrayEndPointTest.java b/jetty-io/src/test/java/org/eclipse/jetty/io/ByteArrayEndPointTest.java
index 1db6908..d23be98 100644
--- a/jetty-io/src/test/java/org/eclipse/jetty/io/ByteArrayEndPointTest.java
+++ b/jetty-io/src/test/java/org/eclipse/jetty/io/ByteArrayEndPointTest.java
@@ -18,6 +18,7 @@
package org.eclipse.jetty.io;
+import static org.hamcrest.Matchers.*;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.instanceOf;
@@ -36,6 +37,8 @@
import org.eclipse.jetty.toolchain.test.annotation.Slow;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.FutureCallback;
+import org.eclipse.jetty.util.log.Log;
+import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.util.thread.Scheduler;
import org.eclipse.jetty.util.thread.TimerScheduler;
import org.junit.After;
@@ -235,6 +238,26 @@
assertEquals(null, fcb.get());
assertEquals(" more.", endp.getOutputString());
}
+
+ /**
+ * Simulate AbstractConnection.ReadCallback.failed()
+ */
+ public static class Closer extends FutureCallback
+ {
+ private EndPoint endp;
+
+ public Closer(EndPoint endp)
+ {
+ this.endp = endp;
+ }
+
+ @Override
+ public void failed(Throwable cause)
+ {
+ endp.close();
+ super.failed(cause);
+ }
+ }
@Slow
@Test
@@ -275,7 +298,7 @@
assertThat(t.getCause(), instanceOf(TimeoutException.class));
}
assertThat(System.currentTimeMillis() - start, greaterThan(idleTimeout / 2));
- assertTrue(endp.isOpen());
+ assertThat("Endpoint open", endp.isOpen(), is(true));
// We need to delay the write timeout test below from the read timeout test above.
// The reason is that the scheduler thread that fails the endPoint WriteFlusher
@@ -298,17 +321,19 @@
assertThat(t.getCause(), instanceOf(TimeoutException.class));
}
assertThat(System.currentTimeMillis() - start, greaterThan(idleTimeout / 2));
- assertTrue(endp.isOpen());
+ assertThat("Endpoint open", endp.isOpen(), is(true));
- // Still no idle close
- Thread.sleep(idleTimeout * 2);
- assertTrue(endp.isOpen());
+ endp.fillInterested(new Closer(endp));
+
+ // Still no idle close (wait half the time)
+ Thread.sleep(idleTimeout / 2);
+ assertThat("Endpoint open", endp.isOpen(), is(true));
// shutdown out
endp.shutdownOutput();
- // idle close
+ // idle close (wait double the time)
Thread.sleep(idleTimeout * 2);
- assertFalse(endp.isOpen());
+ assertThat("Endpoint open", endp.isOpen(), is(false));
}
}
