Merge pull request #4535 from eclipse/jetty-10.0.x-4462-javaxWSClose

Issue #4462 - do not throw IOException on Javax websocket close
This commit is contained in:
Lachlan 2020-02-03 10:22:23 +11:00 committed by GitHub
commit f8219a56cc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 62 additions and 63 deletions

View File

@ -22,7 +22,6 @@ import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.nio.ByteBuffer;
import java.nio.channels.ClosedChannelException;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
@ -300,7 +299,7 @@ public class JavaxWebSocketFrameHandler implements FrameHandler
// Make sure onClose is only notified once.
if (!closeNotified.compareAndSet(false, true))
{
callback.failed(new ClosedChannelException());
callback.succeeded();
return;
}

View File

@ -177,7 +177,7 @@ public class JavaxWebSocketSession implements javax.websocket.Session
* @since JSR356 v1.0
*/
@Override
public void close() throws IOException
public void close()
{
close(new CloseReason(CloseReason.CloseCodes.NO_STATUS_CODE, null));
}
@ -189,11 +189,18 @@ public class JavaxWebSocketSession implements javax.websocket.Session
* @since JSR356 v1.0
*/
@Override
public void close(CloseReason closeReason) throws IOException
public void close(CloseReason closeReason)
{
FutureCallback b = new FutureCallback();
coreSession.close(closeReason.getCloseCode().getCode(), closeReason.getReasonPhrase(), b);
b.block(getBlockingTimeout(), TimeUnit.MILLISECONDS);
try
{
FutureCallback b = new FutureCallback();
coreSession.close(closeReason.getCloseCode().getCode(), closeReason.getReasonPhrase(), b);
b.block(getBlockingTimeout(), TimeUnit.MILLISECONDS);
}
catch (IOException e)
{
LOG.ignore(e);
}
}
private long getBlockingTimeout()

View File

@ -18,9 +18,7 @@
package org.eclipse.jetty.websocket.javax.tests;
import java.io.IOException;
import java.net.URI;
import java.nio.channels.ClosedChannelException;
import java.util.Objects;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
@ -41,13 +39,13 @@ import org.eclipse.jetty.websocket.javax.server.config.JavaxWebSocketServletCont
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.function.Executable;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class JavaxOnCloseTest
@ -161,7 +159,7 @@ public class JavaxOnCloseTest
OnCloseEndpoint serverEndpoint = Objects.requireNonNull(serverEndpoints.poll(5, TimeUnit.SECONDS));
assertTrue(serverEndpoint.openLatch.await(5, TimeUnit.SECONDS));
serverEndpoint.setOnClose((session) -> assertThrows(ClosedChannelException.class, session::close));
serverEndpoint.setOnClose((session) -> assertDoesNotThrow((Executable)session::close));
serverEndpoint.session.close(new CloseReason(CloseCodes.NORMAL_CLOSURE, "first close"));
assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS));
@ -180,9 +178,7 @@ public class JavaxOnCloseTest
assertTrue(serverEndpoint.openLatch.await(5, TimeUnit.SECONDS));
serverEndpoint.setOnClose((session) ->
{
IOException error = assertThrows(IOException.class,
() -> session.close(new CloseReason(CloseCodes.UNEXPECTED_CONDITION, "abnormal close 2")));
assertThat(error.getCause(), instanceOf(ClosedChannelException.class));
assertDoesNotThrow(() -> session.close(new CloseReason(CloseCodes.UNEXPECTED_CONDITION, "abnormal close 2")));
clientEndpoint.unBlockClose();
});
@ -206,16 +202,7 @@ public class JavaxOnCloseTest
throw new RuntimeException("trigger onError from onClose");
});
try
{
clientEndpoint.session.close();
}
catch (IOException e)
{
// Ignore. This only occurs in the rare case where the
// close response is received while we are still sending the message.
}
clientEndpoint.session.close();
assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS));
assertThat(clientEndpoint.closeReason.getCloseCode(), is(CloseCodes.UNEXPECTED_CONDITION));
assertThat(clientEndpoint.closeReason.getReasonPhrase(), containsString("trigger onError from onClose"));

View File

@ -104,53 +104,59 @@ public class SessionTrackingTest
EventSocket clientSocket2 = new EventSocket();
EventSocket clientSocket3 = new EventSocket();
Session session1 = client.connectToServer(clientSocket1, server.getWsUri().resolve("/session-info/1"));
Session serverSession1 = serverSessions.poll(5, TimeUnit.SECONDS);
assertNotNull(serverSession1);
sendTextFrameToAll("openSessions|in-1", session1);
assertThat(clientSocket1.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@in-1).size=1"));
try (Session session1 = client.connectToServer(clientSocket1, server.getWsUri().resolve("/session-info/1")))
{
Session serverSession1 = serverSessions.poll(5, TimeUnit.SECONDS);
assertNotNull(serverSession1);
sendTextFrameToAll("openSessions|in-1", session1);
assertThat(clientSocket1.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@in-1).size=1"));
Session session2 = client.connectToServer(clientSocket2, server.getWsUri().resolve("/session-info/2"));
Session serverSession2 = serverSessions.poll(5, TimeUnit.SECONDS);
assertNotNull(serverSession2);
sendTextFrameToAll("openSessions|in-2", session1, session2);
assertThat(clientSocket1.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@in-2).size=2"));
assertThat(clientSocket2.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@in-2).size=2"));
try (Session session2 = client.connectToServer(clientSocket2, server.getWsUri().resolve("/session-info/2")))
{
Session serverSession2 = serverSessions.poll(5, TimeUnit.SECONDS);
assertNotNull(serverSession2);
sendTextFrameToAll("openSessions|in-2", session1, session2);
assertThat(clientSocket1.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@in-2).size=2"));
assertThat(clientSocket2.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@in-2).size=2"));
Session session3 = client.connectToServer(clientSocket3, server.getWsUri().resolve("/session-info/3"));
Session serverSession3 = serverSessions.poll(5, TimeUnit.SECONDS);
assertNotNull(serverSession3);
sendTextFrameToAll("openSessions|in-3", session1, session2, session3);
assertThat(clientSocket1.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@in-3).size=3"));
assertThat(clientSocket2.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@in-3).size=3"));
assertThat(clientSocket3.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@in-3).size=3"));
try (Session session3 = client.connectToServer(clientSocket3, server.getWsUri().resolve("/session-info/3")))
{
Session serverSession3 = serverSessions.poll(5, TimeUnit.SECONDS);
assertNotNull(serverSession3);
sendTextFrameToAll("openSessions|in-3", session1, session2, session3);
assertThat(clientSocket1.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@in-3).size=3"));
assertThat(clientSocket2.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@in-3).size=3"));
assertThat(clientSocket3.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@in-3).size=3"));
sendTextFrameToAll("openSessions|lvl-3", session1, session2, session3);
assertThat(clientSocket1.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@lvl-3).size=3"));
assertThat(clientSocket2.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@lvl-3).size=3"));
assertThat(clientSocket3.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@lvl-3).size=3"));
sendTextFrameToAll("openSessions|lvl-3", session1, session2, session3);
assertThat(clientSocket1.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@lvl-3).size=3"));
assertThat(clientSocket2.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@lvl-3).size=3"));
assertThat(clientSocket3.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@lvl-3).size=3"));
// assert session is closed, and we have received the notification from the SessionListener
session3.close();
assertThat(server.getTrackingListener().getClosedSessions().poll(5, TimeUnit.SECONDS), sameInstance(serverSession3));
assertTrue(clientSocket3.closeLatch.await(5, TimeUnit.SECONDS));
// assert session is closed, and we have received the notification from the SessionListener
session3.close();
assertThat(server.getTrackingListener().getClosedSessions().poll(5, TimeUnit.SECONDS), sameInstance(serverSession3));
assertTrue(clientSocket3.closeLatch.await(5, TimeUnit.SECONDS));
}
sendTextFrameToAll("openSessions|lvl-2", session1, session2);
assertThat(clientSocket1.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@lvl-2).size=2"));
assertThat(clientSocket2.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@lvl-2).size=2"));
sendTextFrameToAll("openSessions|lvl-2", session1, session2);
assertThat(clientSocket1.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@lvl-2).size=2"));
assertThat(clientSocket2.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@lvl-2).size=2"));
// assert session is closed, and we have received the notification from the SessionListener
session2.close();
assertThat(server.getTrackingListener().getClosedSessions().poll(5, TimeUnit.SECONDS), sameInstance(serverSession2));
assertTrue(clientSocket2.closeLatch.await(5, TimeUnit.SECONDS));
// assert session is closed, and we have received the notification from the SessionListener
session2.close();
assertThat(server.getTrackingListener().getClosedSessions().poll(5, TimeUnit.SECONDS), sameInstance(serverSession2));
assertTrue(clientSocket2.closeLatch.await(5, TimeUnit.SECONDS));
}
sendTextFrameToAll("openSessions|lvl-1", session1);
assertThat(clientSocket1.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@lvl-1).size=1"));
sendTextFrameToAll("openSessions|lvl-1", session1);
assertThat(clientSocket1.textMessages.poll(5, TimeUnit.SECONDS), is("openSessions(@lvl-1).size=1"));
// assert session is closed, and we have received the notification from the SessionListener
session1.close();
assertThat(server.getTrackingListener().getClosedSessions().poll(5, TimeUnit.SECONDS), sameInstance(serverSession1));
assertTrue(clientSocket1.closeLatch.await(5, TimeUnit.SECONDS));
// assert session is closed, and we have received the notification from the SessionListener
session1.close();
assertThat(server.getTrackingListener().getClosedSessions().poll(5, TimeUnit.SECONDS), sameInstance(serverSession1));
assertTrue(clientSocket1.closeLatch.await(5, TimeUnit.SECONDS));
}
}
private static void sendTextFrameToAll(String msg, Session... sessions) throws IOException