Merge pull request #3637 from lachlan-roberts/jetty-9.4.x-3498-websocket-suspend

Issue #3498 - jetty 9.4 websocket suspend for incoming frames
This commit is contained in:
Lachlan 2019-06-04 13:42:24 +10:00 committed by GitHub
commit ac8303c45e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 335 additions and 222 deletions

View File

@ -0,0 +1,202 @@
//
// ========================================================================
// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//
package org.eclipse.jetty.websocket.tests;
import java.io.IOException;
import java.net.URI;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.eclipse.jetty.websocket.api.Session;
import org.eclipse.jetty.websocket.api.StatusCode;
import org.eclipse.jetty.websocket.api.SuspendToken;
import org.eclipse.jetty.websocket.api.annotations.WebSocket;
import org.eclipse.jetty.websocket.client.WebSocketClient;
import org.eclipse.jetty.websocket.servlet.WebSocketServlet;
import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class SuspendResumeTest
{
@WebSocket
public static class SuspendSocket extends EventSocket
{
volatile SuspendToken suspendToken = null;
@Override
public void onMessage(String message) throws IOException
{
if ("suspend".equals(message))
suspendToken = session.suspend();
super.onMessage(message);
}
@Override
public void onError(Throwable cause)
{
super.onError(cause);
cause.printStackTrace();
}
}
public class UpgradeServlet extends WebSocketServlet
{
@Override
public void configure(WebSocketServletFactory factory)
{
factory.setCreator(((req, resp) -> serverSocket));
}
}
private Server server = new Server();
private WebSocketClient client = new WebSocketClient();
private SuspendSocket serverSocket = new SuspendSocket();
private ServerConnector connector;
@BeforeEach
public void start() throws Exception
{
connector = new ServerConnector(server);
server.addConnector(connector);
ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS);
contextHandler.setContextPath("/");
server.setHandler(contextHandler);
contextHandler.addServlet(new ServletHolder(new UpgradeServlet()), "/suspend");
server.start();
client.start();
}
@AfterEach
public void stop() throws Exception
{
client.stop();
server.stop();
}
@Test
public void testSuspendWhenProcessingFrame() throws Exception
{
URI uri = new URI("ws://localhost:"+connector.getLocalPort()+"/suspend");
EventSocket clientSocket = new EventSocket();
Future<Session> connect = client.connect(clientSocket, uri);
connect.get(5, TimeUnit.SECONDS);
clientSocket.session.getRemote().sendString("suspend");
clientSocket.session.getRemote().sendString("suspend");
clientSocket.session.getRemote().sendString("hello world");
assertThat(serverSocket.receivedMessages.poll(5, TimeUnit.SECONDS), is("suspend"));
assertNull(serverSocket.receivedMessages.poll(1, TimeUnit.SECONDS));
serverSocket.suspendToken.resume();
assertThat(serverSocket.receivedMessages.poll(5, TimeUnit.SECONDS), is("suspend"));
assertNull(serverSocket.receivedMessages.poll(1, TimeUnit.SECONDS));
serverSocket.suspendToken.resume();
assertThat(serverSocket.receivedMessages.poll(5, TimeUnit.SECONDS), is("hello world"));
assertNull(serverSocket.receivedMessages.poll(1, TimeUnit.SECONDS));
// make sure both sides are closed
clientSocket.session.close();
assertTrue(clientSocket.closed.await(5, TimeUnit.SECONDS));
assertTrue(serverSocket.closed.await(5, TimeUnit.SECONDS));
// check we closed normally
assertThat(clientSocket.closeCode, is(StatusCode.NORMAL));
assertThat(serverSocket.closeCode, is(StatusCode.NORMAL));
}
@Test
public void testExternalSuspend() throws Exception
{
URI uri = new URI("ws://localhost:"+connector.getLocalPort()+"/suspend");
EventSocket clientSocket = new EventSocket();
Future<Session> connect = client.connect(clientSocket, uri);
connect.get(5, TimeUnit.SECONDS);
// verify connection by sending a message from server to client
assertTrue(serverSocket.open.await(5, TimeUnit.SECONDS));
serverSocket.session.getRemote().sendString("verification");
assertThat(clientSocket.receivedMessages.poll(5, TimeUnit.SECONDS), is("verification"));
// suspend the client so that no read events occur
SuspendToken suspendToken = clientSocket.session.suspend();
// verify client can still send messages
clientSocket.session.getRemote().sendString("message-from-client");
assertThat(serverSocket.receivedMessages.poll(5, TimeUnit.SECONDS), is("message-from-client"));
// the message is not received as it is suspended
serverSocket.session.getRemote().sendString("message-from-server");
assertNull(clientSocket.receivedMessages.poll(2, TimeUnit.SECONDS));
// client should receive message after it resumes
suspendToken.resume();
assertThat(clientSocket.receivedMessages.poll(5, TimeUnit.SECONDS), is("message-from-server"));
// make sure both sides are closed
clientSocket.session.close();
assertTrue(clientSocket.closed.await(5, TimeUnit.SECONDS));
assertTrue(serverSocket.closed.await(5, TimeUnit.SECONDS));
// check we closed normally
assertThat(clientSocket.closeCode, is(StatusCode.NORMAL));
assertThat(serverSocket.closeCode, is(StatusCode.NORMAL));
}
@Test
public void testSuspendAfterClose() throws Exception
{
URI uri = new URI("ws://localhost:"+connector.getLocalPort()+"/suspend");
EventSocket clientSocket = new EventSocket();
Future<Session> connect = client.connect(clientSocket, uri);
connect.get(5, TimeUnit.SECONDS);
// verify connection by sending a message from server to client
assertTrue(serverSocket.open.await(5, TimeUnit.SECONDS));
serverSocket.session.getRemote().sendString("verification");
assertThat(clientSocket.receivedMessages.poll(5, TimeUnit.SECONDS), is("verification"));
// make sure both sides are closed
clientSocket.session.close();
assertTrue(clientSocket.closed.await(5, TimeUnit.SECONDS));
assertTrue(serverSocket.closed.await(5, TimeUnit.SECONDS));
// check we closed normally
assertThat(clientSocket.closeCode, is(StatusCode.NORMAL));
assertThat(serverSocket.closeCode, is(StatusCode.NORMAL));
// suspend the client so that no read events occur
SuspendToken suspendToken = clientSocket.session.suspend();
suspendToken.resume();
}
}

View File

@ -236,17 +236,22 @@ public class Parser
}
public void parse(ByteBuffer buffer) throws WebSocketException
{
while (buffer.hasRemaining())
parseSingleFrame(buffer);
}
public void parseSingleFrame(ByteBuffer buffer) throws WebSocketException
{
if (buffer.remaining() <= 0)
{
return;
}
try
{
bytesIn.add(buffer.remaining());
int startingBytes = buffer.remaining();
// parse through all the frames in the buffer
while (parseFrame(buffer))
// attempt to parse a frame from the buffer
if (parseFrame(buffer))
{
if (LOG.isDebugEnabled())
LOG.debug("{} Parsed Frame: {}",policy.getBehavior(),frame);
@ -259,20 +264,18 @@ public class Parser
}
reset();
}
}
catch (WebSocketException e)
{
buffer.position(buffer.limit()); // consume remaining
reset();
// need to throw for proper close behavior in connection
throw e;
bytesIn.add(startingBytes - buffer.remaining());
}
catch (Throwable t)
{
buffer.position(buffer.limit()); // consume remaining
reset();
// need to throw for proper close behavior in connection
throw new WebSocketException(t);
if (t instanceof WebSocketException)
throw t;
else
throw new WebSocketException(t);
}
}

View File

@ -424,27 +424,34 @@ public abstract class AbstractWebSocketConnection extends AbstractConnection imp
LOG.debug("{} onFillable()", policy.getBehavior());
}
stats.countOnFillableEvents.incrementAndGet();
ByteBuffer buffer = bufferPool.acquire(getInputBufferSize(), true);
onFillable(buffer);
}
private void onFillable(ByteBuffer buffer)
{
if (LOG.isDebugEnabled())
{
LOG.debug("{} onFillable(ByteBuffer): {}", policy.getBehavior(), buffer);
}
try
{
if (readMode == ReadMode.PARSE)
{
readMode = readParse(buffer);
}
else
{
readMode = readDiscard(buffer);
}
}
finally
catch (Throwable t)
{
bufferPool.release(buffer);
throw t;
}
if (readMode == ReadMode.EOF)
{
bufferPool.release(buffer);
readState.eof();
// Handle case where the remote connection was abruptly terminated without a close frame
@ -453,6 +460,7 @@ public abstract class AbstractWebSocketConnection extends AbstractConnection imp
}
else if (!readState.suspend())
{
bufferPool.release(buffer);
fillInterested();
}
}
@ -557,8 +565,24 @@ public abstract class AbstractWebSocketConnection extends AbstractConnection imp
try
{
// Process the content from the Endpoint next
while (true) // TODO: should this honor the LogicalConnection.suspend() ?
while (true)
{
// We may start with a non empty buffer, consume before filling
while (buffer.hasRemaining())
{
if (readState.suspendParse(buffer))
{
if (LOG.isDebugEnabled())
{
LOG.debug("suspending parse {}", buffer);
}
return ReadMode.PARSE;
}
else
parser.parseSingleFrame(buffer);
}
int filled = endPoint.fill(buffer);
if (filled < 0)
{
@ -578,7 +602,6 @@ public abstract class AbstractWebSocketConnection extends AbstractConnection imp
{
LOG.debug("Filled {} bytes - {}", filled, BufferUtil.toDetailString(buffer));
}
parser.parse(buffer);
}
}
catch (Throwable t)
@ -591,10 +614,22 @@ public abstract class AbstractWebSocketConnection extends AbstractConnection imp
@Override
public void resume()
{
if (readState.resume())
ByteBuffer resume = readState.resume();
if (resume == null)
{
fillInterested();
}
else if (resume != ReadState.NO_ACTION)
{
onFillable(resume);
}
}
@Override
public SuspendToken suspend()
{
readState.suspending();
return this;
}
/**
@ -625,13 +660,6 @@ public abstract class AbstractWebSocketConnection extends AbstractConnection imp
getEndPoint().setIdleTimeout(ms);
}
@Override
public SuspendToken suspend()
{
readState.suspending();
return this;
}
@Override
public String dumpSelf()
{

View File

@ -18,21 +18,31 @@
package org.eclipse.jetty.websocket.common.io;
import java.util.concurrent.atomic.AtomicReference;
import java.nio.ByteBuffer;
import org.eclipse.jetty.util.BufferUtil;
class ReadState
{
private final AtomicReference<State> state = new AtomicReference<>(State.READING);
public final static ByteBuffer NO_ACTION = BufferUtil.EMPTY_BUFFER;
private State state = State.READING;
private ByteBuffer buffer;
boolean isReading()
{
return state.get() == State.READING;
synchronized (this)
{
return state == State.READING;
}
}
boolean isSuspended()
{
State current = state.get();
return current == State.SUSPENDED || current == State.EOF;
synchronized (this)
{
return state == State.SUSPENDED || state == State.EOF;
}
}
/**
@ -42,19 +52,35 @@ class ReadState
*/
boolean suspending()
{
while (true)
synchronized (this)
{
State current = state.get();
switch (current)
switch (state)
{
case READING:
if (state.compareAndSet(current, State.SUSPENDING))
return true;
break;
state = State.SUSPENDING;
return true;
case EOF:
return false;
default:
throw new IllegalStateException(toString(current));
throw new IllegalStateException(toString(state));
}
}
}
public boolean suspendParse(ByteBuffer buffer)
{
synchronized (this)
{
switch (state)
{
case READING:
return false;
case SUSPENDING:
this.buffer = buffer;
this.state = State.SUSPENDED;
return true;
default:
throw new IllegalStateException(toString(state));
}
}
}
@ -66,54 +92,59 @@ class ReadState
*/
boolean suspend()
{
while (true)
synchronized (this)
{
State current = state.get();
switch (current)
switch (state)
{
case READING:
return false;
case SUSPENDING:
if (state.compareAndSet(current, State.SUSPENDED))
return true;
break;
state = State.SUSPENDED;
return true;
case SUSPENDED:
if (buffer == null)
throw new IllegalStateException();
return true;
case EOF:
return true;
default:
throw new IllegalStateException(toString(current));
throw new IllegalStateException(toString(state));
}
}
}
/**
* @return true if reads from the connection were suspended and should now resume.
* @return a ByteBuffer to finish processing, or null if we should register fillInterested
* If return value is {@link BufferUtil#EMPTY_BUFFER} no action should be taken.
*/
boolean resume()
ByteBuffer resume()
{
while (true)
synchronized (this)
{
State current = state.get();
switch (current)
switch (state)
{
case SUSPENDING:
if (state.compareAndSet(current, State.READING))
return false;
break;
state = State.READING;
return NO_ACTION;
case SUSPENDED:
if (state.compareAndSet(current, State.READING))
return true;
break;
state = State.READING;
ByteBuffer bb = buffer;
buffer = null;
return bb;
case EOF:
return false;
return NO_ACTION;
default:
throw new IllegalStateException(toString(current));
throw new IllegalStateException(toString(state));
}
}
}
void eof()
{
state.set(State.EOF);
synchronized (this)
{
state = State.EOF;
}
}
private String toString(State state)
@ -124,7 +155,10 @@ class ReadState
@Override
public String toString()
{
return toString(state.get());
synchronized (this)
{
return toString(state);
}
}
private enum State
@ -147,6 +181,6 @@ class ReadState
/**
* Won't read from the connection (terminal state).
*/
EOF,
EOF
}
}

View File

@ -22,6 +22,7 @@ import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
@ -49,7 +50,7 @@ public class ReadStateTest
assertTrue(readState.suspending());
assertThat("Suspending doesn't take effect immediately", readState.isSuspended(), is(false));
assertThat("Resume from suspending requires no followup", readState.resume(), is(false));
assertThat("Resume from suspending requires no followup", readState.resume(), is(ReadState.NO_ACTION));
assertThat("Resume from suspending requires no followup", readState.isSuspended(), is(false));
assertThat("Suspending was discarded", readState.suspend(), is(false));
@ -68,7 +69,7 @@ public class ReadStateTest
assertThat("Suspended", readState.suspend(), is(true));
assertThat("Suspended", readState.isSuspended(), is(true));
assertThat("Resumed", readState.resume(), is(true));
assertNull(readState.resume(), "Resumed");
assertThat("Resumed", readState.isSuspended(), is(false));
}
@ -88,7 +89,7 @@ public class ReadStateTest
assertThat(readState.suspend(), is(true));
assertThat(readState.isSuspended(), is(true));
assertThat(readState.resume(), is(false));
assertThat(readState.resume(), is(ReadState.NO_ACTION));
assertThat(readState.isSuspended(), is(true));
}
}

View File

@ -1,155 +0,0 @@
//
// ========================================================================
// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//
package org.eclipse.jetty.websocket.server;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.fail;
import java.util.concurrent.Future;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;
import org.eclipse.jetty.websocket.api.Session;
import org.eclipse.jetty.websocket.api.SuspendToken;
import org.eclipse.jetty.websocket.api.WriteCallback;
import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect;
import org.eclipse.jetty.websocket.api.annotations.OnWebSocketMessage;
import org.eclipse.jetty.websocket.api.annotations.WebSocket;
import org.eclipse.jetty.websocket.common.WebSocketFrame;
import org.eclipse.jetty.websocket.common.frames.TextFrame;
import org.eclipse.jetty.websocket.common.test.BlockheadClient;
import org.eclipse.jetty.websocket.common.test.BlockheadClientRequest;
import org.eclipse.jetty.websocket.common.test.BlockheadConnection;
import org.eclipse.jetty.websocket.common.test.Timeouts;
import org.eclipse.jetty.websocket.servlet.ServletUpgradeRequest;
import org.eclipse.jetty.websocket.servlet.ServletUpgradeResponse;
import org.eclipse.jetty.websocket.servlet.WebSocketCreator;
import org.eclipse.jetty.websocket.servlet.WebSocketServlet;
import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
public class SuspendResumeTest
{
@WebSocket
public static class EchoSocket
{
private Session session;
@OnWebSocketConnect
public void onConnect(Session session)
{
this.session = session;
}
@OnWebSocketMessage
public void onMessage(String message)
{
SuspendToken suspendToken = this.session.suspend();
this.session.getRemote().sendString(message,
new WriteCallback()
{
@Override
public void writeSuccess()
{
suspendToken.resume();
}
@Override
public void writeFailed(Throwable t)
{
fail(t);
}
});
}
}
public static class EchoCreator implements WebSocketCreator
{
@Override
public Object createWebSocket(ServletUpgradeRequest req, ServletUpgradeResponse resp)
{
return new EchoSocket();
}
}
public static class EchoServlet extends WebSocketServlet
{
private static final long serialVersionUID = 1L;
@Override
public void configure(WebSocketServletFactory factory)
{
factory.setCreator(new EchoCreator());
}
}
private static SimpleServletServer server;
private static BlockheadClient client;
@BeforeAll
public static void startServer() throws Exception
{
server = new SimpleServletServer(new EchoServlet());
server.start();
}
@BeforeAll
public static void startClient() throws Exception
{
client = new BlockheadClient();
client.setIdleTimeout(TimeUnit.SECONDS.toMillis(2));
client.start();
}
@AfterAll
public static void stopClient() throws Exception
{
client.stop();
}
@AfterAll
public static void stopServer()
{
server.stop();
}
@Test
public void testSuspendResume() throws Exception
{
BlockheadClientRequest request = client.newWsRequest(server.getServerUri());
Future<BlockheadConnection> connFut = request.sendAsync();
try (BlockheadConnection clientConn = connFut.get(Timeouts.CONNECT, Timeouts.CONNECT_UNIT))
{
clientConn.write(new TextFrame().setPayload("echo1"));
clientConn.write(new TextFrame().setPayload("echo2"));
LinkedBlockingQueue<WebSocketFrame> frames = clientConn.getFrameQueue();
WebSocketFrame tf = frames.poll(Timeouts.POLL_EVENT, Timeouts.POLL_EVENT_UNIT);
assertThat(EchoSocket.class.getSimpleName() + ".onMessage()", tf.getPayloadAsUTF8(), is("echo1"));
tf = frames.poll(Timeouts.POLL_EVENT, Timeouts.POLL_EVENT_UNIT);
assertThat(EchoSocket.class.getSimpleName() + ".onMessage()", tf.getPayloadAsUTF8(), is("echo2"));
}
}
}