From bc87cf0ab422eec436944866de4e280d9730d516 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 15 Jun 2016 10:32:10 +1000 Subject: [PATCH 1/2] Revert "Debug #624" This reverts commit b40c2ae66fe223b29d490b2993c6e919bc56ea3b. --- .../org/eclipse/jetty/server/HttpChannel.java | 25 ------------------- .../jetty/server/HttpChannelState.java | 4 +-- .../org/eclipse/jetty/server/HttpInput.java | 2 +- .../org/eclipse/jetty/server/HttpOutput.java | 6 ++--- 4 files changed, 6 insertions(+), 31 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java index c065a1d246d..af83928439a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java @@ -265,31 +265,6 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor { handle(); } - - // TODO Revert once #624 debugged - private static ThreadLocal __dispatchedFrom=new ThreadLocal<>(); - public static Throwable getDispatchedFrom() { return __dispatchedFrom.get(); } - public Runnable getRunnable() - { - Throwable _dispatched = new Throwable(); - return new Runnable() - { - @Override - public void run() - { - try - { - __dispatchedFrom.set(_dispatched); - handle(); - } - finally - { - __dispatchedFrom.set(null); - } - } - }; - } - /** * @return True if the channel is ready to continue handling (ie it is not suspended) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java index 7347d9eb76b..fa87e1c1169 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java @@ -592,7 +592,7 @@ public class HttpChannelState cancelTimeout(event); if (handle) - runInContext(event,_channel.getRunnable()); + runInContext(event,_channel); } public void errorComplete() @@ -748,7 +748,7 @@ public class HttpChannelState protected void scheduleDispatch() { - _channel.execute(_channel.getRunnable()); + _channel.execute(_channel); } protected void scheduleTimeout(AsyncContextEvent event) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java index d6c856f1cda..293887cb77a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java @@ -121,7 +121,7 @@ public class HttpInput extends ServletInputStream implements Runnable { HttpChannel channel = _channelState.getHttpChannel(); Executor executor = channel.getConnector().getServer().getThreadPool(); - executor.execute(channel.getRunnable()); + executor.execute(channel); } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java index 2afd0056d51..dc886455a1f 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java @@ -838,7 +838,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable { _writeListener = writeListener; if (_channel.getState().onWritePossible()) - _channel.execute(_channel.getRunnable()); + _channel.execute(_channel); } else throw new IllegalStateException(); @@ -983,7 +983,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable if (!_state.compareAndSet(OutputState.UNREADY, OutputState.READY)) continue; if (_channel.getState().onWritePossible()) - _channel.execute(_channel.getRunnable()); + _channel.execute(_channel); break; case CLOSED: @@ -1001,7 +1001,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable { _onError=e==null?new IOException():e; if (_channel.getState().onWritePossible()) - _channel.execute(_channel.getRunnable()); + _channel.execute(_channel); } } From f724d36646af3fc53bf6da3415ad77a83a5cdeba Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 15 Jun 2016 10:38:40 +1000 Subject: [PATCH 2/2] Cleanup #624 removed excess test code --- .../jetty/server/LocalAsyncContextTest.java | 105 ++++++------------ 1 file changed, 31 insertions(+), 74 deletions(-) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/LocalAsyncContextTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/LocalAsyncContextTest.java index 7eb8eace336..a54489bee36 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/LocalAsyncContextTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/LocalAsyncContextTest.java @@ -43,10 +43,8 @@ import org.junit.Test; public class LocalAsyncContextTest { - private final Locker _completeLock = new Locker(); - private Throwable __completed; - private Throwable __dispatched; - private final AtomicReference __completed1 = new AtomicReference<>(); + private final AtomicReference _completed0 = new AtomicReference<>(); + private final AtomicReference _completed1 = new AtomicReference<>(); protected Server _server; protected SuspendHandler _handler; protected Connector _connector; @@ -65,11 +63,8 @@ public class LocalAsyncContextTest _server.setHandler(session); _server.start(); - try (Locker.Lock lock = _completeLock.lock()) - { - __completed = null; - } - __completed1.set(null); + _completed0.set(null); + _completed1.set(null); } protected Connector initConnector() @@ -95,14 +90,8 @@ public class LocalAsyncContextTest response = process(null); check(response, "TIMEOUT"); - spinAssertEquals(1, () -> - { - try (Locker.Lock lock = _completeLock.lock()) - { - return __completed == null ? 0 : 1; - } - }); - spinAssertEquals(1, () -> __completed1.get() == null ? 0 : 1); + spinAssertEquals(1, () -> _completed0.get() == null ? 0 : 1); + spinAssertEquals(1, () -> _completed1.get() == null ? 0 : 1); } @Test @@ -231,14 +220,8 @@ public class LocalAsyncContextTest response = process(null); check(response, "STARTASYNC", "DISPATCHED", "startasync", "STARTASYNC2", "DISPATCHED"); - spinAssertEquals(1, () -> - { - try (Locker.Lock lock = _completeLock.lock()) - { - return __completed == null ? 0 : 1; - } - }); - spinAssertEquals(0, () -> __completed1.get() == null ? 0 : 1); + spinAssertEquals(1, () -> _completed0.get() == null ? 0 : 1); + spinAssertEquals(0, () -> _completed1.get() == null ? 0 : 1); } protected void check(String response, String... content) @@ -490,56 +473,30 @@ public class LocalAsyncContextTest public void onComplete(AsyncEvent event) throws IOException { Throwable complete = new Throwable(); - Throwable dispatched = HttpChannel.getDispatchedFrom(); - try (Locker.Lock lock = _completeLock.lock()) + if (!_completed0.compareAndSet(null, complete)) { - if (__completed == null) - { - __completed = complete; - __dispatched = dispatched; - } - else - { - System.err.println("First onCompleted dispatched from:"); - if (__dispatched != null) - __dispatched.printStackTrace(); - System.err.println("First onCompleted:"); - __completed.printStackTrace(); - System.err.println("Second onCompleted dispatched from:"); - if (dispatched != null) - dispatched.printStackTrace(); - complete.printStackTrace(); - throw new IllegalStateException(); - } + System.err.println("First onCompleted:"); + _completed0.get().printStackTrace(); + System.err.println("First onCompleted:"); + complete.printStackTrace(); + _completed0.set(null); + throw new IllegalStateException(); } } @Override public void onError(AsyncEvent event) throws IOException { - Throwable complete = new Throwable(); - Throwable dispatched = HttpChannel.getDispatchedFrom(); - try (Locker.Lock lock = _completeLock.lock()) - { - if (__completed == null) - { - __completed = complete; - __dispatched = dispatched; - } - else - { - System.err.println("First onCompleted dispatched from:"); - if (__dispatched != null) - __dispatched.printStackTrace(); - System.err.println("First onCompleted:"); - __completed.printStackTrace(); - System.err.println("Second onCompleted dispatched from:"); - if (dispatched != null) - dispatched.printStackTrace(); - complete.printStackTrace(); - throw new IllegalStateException(); - } - } + Throwable complete = new Throwable(); + if (!_completed0.compareAndSet(null, complete)) + { + System.err.println("First onCompleted:"); + _completed0.get().printStackTrace(); + System.err.println("First onCompleted:"); + complete.printStackTrace(); + _completed0.set(null); + throw new IllegalStateException(); + } } @Override @@ -563,11 +520,11 @@ public class LocalAsyncContextTest public void onComplete(AsyncEvent event) throws IOException { Throwable complete = new Throwable(); - if (!__completed1.compareAndSet(null, complete)) + if (!_completed1.compareAndSet(null, complete)) { - __completed1.get().printStackTrace(); + _completed1.get().printStackTrace(); complete.printStackTrace(); - __completed1.set(null); + _completed1.set(null); throw new IllegalStateException(); } } @@ -576,11 +533,11 @@ public class LocalAsyncContextTest public void onError(AsyncEvent event) throws IOException { Throwable complete = new Throwable(); - if (!__completed1.compareAndSet(null, complete)) + if (!_completed1.compareAndSet(null, complete)) { - __completed1.get().printStackTrace(); + _completed1.get().printStackTrace(); complete.printStackTrace(); - __completed1.set(null); + _completed1.set(null); throw new IllegalStateException(); } }