gemmellr commented on a change in pull request #45:
URL: https://github.com/apache/qpid-jms/pull/45#discussion_r823569330
##########
File path:
qpid-jms-client/src/test/java/org/apache/qpid/jms/transports/netty/NettyTcpTransportTest.java
##########
@@ -394,6 +399,123 @@ public void testZeroSizedSentNoErrors() throws Exception {
assertTrue(data.isEmpty());
}
+ @Test(timeout = 60 * 1000)
+ public void testCannotDereferenceSharedClosedEventLoopGroup() throws
Exception {
+ try (NettyEchoServer server = createEchoServer(createServerOptions()))
{
+
+ server.start();
+
+ int port = server.getServerPort();
+ URI serverLocation = new URI("tcp://localhost:" + port);
+ final TransportOptions sharedTransportOptions =
createClientOptions();
+ sharedTransportOptions.setUseKQueue(false);
+ sharedTransportOptions.setUseEpoll(false);
+ sharedTransportOptions.setSharedEventLoopThreads(1);
+
+ EventLoopGroupRef groupRef = null;
+ Transport nioTransport = createConnectedTransport(serverLocation,
sharedTransportOptions);
+ try {
+ groupRef = getGroupRef(nioTransport);
+ assertNotNull(groupRef.group());
+ } finally {
+ nioTransport.close();
+ }
+
+ server.stop();
+
+ try {
+ groupRef.group();
+ fail("Should have thrown ISE due to being closed");
+ } catch (IllegalStateException expected) {
+ // Ignore
+ } catch (Throwable unexpected) {
+ fail("Should have thrown IllegalStateException");
+ }
+ }
+
+ assertTrue(!transportClosed); // Normal shutdown does not trigger the
event.
+ assertTrue(exceptions.isEmpty());
+ assertTrue(data.isEmpty());
+ }
+
+ @Test(timeout = 60 * 1000)
+ public void testSharedEventLoopGroups() throws Exception {
+ try (NettyEchoServer server = createEchoServer(createServerOptions()))
{
+ server.start();
+
+ int port = server.getServerPort();
+ URI serverLocation = new URI("tcp://localhost:" + port);
+ final TransportOptions sharedTransportOptions =
createClientOptions();
+ sharedTransportOptions.setUseKQueue(false);
+ sharedTransportOptions.setUseEpoll(false);
+ sharedTransportOptions.setSharedEventLoopThreads(1);
+ Transport sharedNioTransport1 =
createConnectedTransport(serverLocation, sharedTransportOptions);
+ Transport sharedNioTransport2 =
createConnectedTransport(serverLocation, sharedTransportOptions);
Review comment:
The test should 'collect' these as they are created and ensure they get
shut down at the end. If any assertion fails in the 'wrong place' it could leak
them, which could impact other tests due to the sharing. Same for other test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
