Skip to content

Commit

Permalink
Correctly close EmbeddedChannel and release buffers in `SnappyFrame…
Browse files Browse the repository at this point in the history
…DecoderTest` (netty#9851)

Motivation:

We did not correctly close the `EmbeddedChannel` which would lead to not have `handlerRemoved(...)` called. This can lead to leaks. Beside this we also did not correctly consume produced data which could also show up as a leak.

Modifications:

- Always call `EmbeddedChannel.finish()`
- Ensure we consume all produced data and release it

Result:

No more leaks in test. This showed up in netty#9850 (comment).
  • Loading branch information
normanmaurer authored Dec 6, 2019
1 parent e69c417 commit d6638d5
Showing 1 changed file with 26 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ public void testReservedUnskippableChunkTypeCausesError() throws Exception {
0x03, 0x01, 0x00, 0x00, 0x00
});

channel.writeInbound(in);
assertFalse(channel.writeInbound(in));
assertFalse(channel.finish());
}

@Test(expected = DecompressionException.class)
Expand All @@ -46,7 +47,8 @@ public void testInvalidStreamIdentifierLength() throws Exception {
-0x80, 0x05, 0x00, 0x00, 'n', 'e', 't', 't', 'y'
});

channel.writeInbound(in);
assertFalse(channel.writeInbound(in));
assertFalse(channel.finish());
}

@Test(expected = DecompressionException.class)
Expand All @@ -55,7 +57,8 @@ public void testInvalidStreamIdentifierValue() throws Exception {
(byte) 0xff, 0x06, 0x00, 0x00, 's', 'n', 'e', 't', 't', 'y'
});

channel.writeInbound(in);
assertFalse(channel.writeInbound(in));
assertFalse(channel.finish());
}

@Test(expected = DecompressionException.class)
Expand All @@ -73,7 +76,8 @@ public void testUncompressedDataBeforeStreamIdentifier() throws Exception {
0x01, 0x05, 0x00, 0x00, 'n', 'e', 't', 't', 'y'
});

channel.writeInbound(in);
assertFalse(channel.writeInbound(in));
assertFalse(channel.finish());
}

@Test(expected = DecompressionException.class)
Expand All @@ -82,7 +86,8 @@ public void testCompressedDataBeforeStreamIdentifier() throws Exception {
0x00, 0x05, 0x00, 0x00, 'n', 'e', 't', 't', 'y'
});

channel.writeInbound(in);
assertFalse(channel.writeInbound(in));
assertFalse(channel.finish());
}

@Test
Expand All @@ -92,10 +97,11 @@ public void testReservedSkippableSkipsInput() throws Exception {
-0x7f, 0x05, 0x00, 0x00, 'n', 'e', 't', 't', 'y'
});

channel.writeInbound(in);
assertFalse(channel.writeInbound(in));
assertNull(channel.readInbound());

assertFalse(in.isReadable());
assertFalse(channel.finish());
}

@Test
Expand All @@ -105,14 +111,15 @@ public void testUncompressedDataAppendsToOut() throws Exception {
0x01, 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 'n', 'e', 't', 't', 'y'
});

channel.writeInbound(in);
assertTrue(channel.writeInbound(in));

ByteBuf expected = Unpooled.wrappedBuffer(new byte[] { 'n', 'e', 't', 't', 'y' });
ByteBuf actual = channel.readInbound();
assertEquals(expected, actual);

expected.release();
actual.release();
assertFalse(channel.finish());
}

@Test
Expand All @@ -125,7 +132,7 @@ public void testCompressedDataDecodesAndAppendsToOut() throws Exception {
0x6e, 0x65, 0x74, 0x74, 0x79 // "netty"
});

channel.writeInbound(in);
assertTrue(channel.writeInbound(in));

ByteBuf expected = Unpooled.wrappedBuffer(new byte[] { 'n', 'e', 't', 't', 'y' });
ByteBuf actual = channel.readInbound();
Expand All @@ -134,6 +141,7 @@ public void testCompressedDataDecodesAndAppendsToOut() throws Exception {

expected.release();
actual.release();
assertFalse(channel.finish());
}

// The following two tests differ in only the checksum provided for the literal
Expand All @@ -149,7 +157,8 @@ public void testInvalidChecksumThrowsException() throws Exception {
0x01, 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 'n', 'e', 't', 't', 'y'
});

channel.writeInbound(in);
assertFalse(channel.writeInbound(in));
assertFalse(channel.finish());
}

@Test
Expand All @@ -162,6 +171,13 @@ public void testInvalidChecksumDoesNotThrowException() throws Exception {
0x01, 0x09, 0x00, 0x00, 0x6f, -0x68, -0x7e, -0x5e, 'n', 'e', 't', 't', 'y'
});

channel.writeInbound(in);
assertTrue(channel.writeInbound(in));
ByteBuf expected = Unpooled.wrappedBuffer(new byte[] { 'n', 'e', 't', 't', 'y' });
ByteBuf actual = channel.readInbound();
assertEquals(expected, actual);

expected.release();
actual.release();
assertFalse(channel.finish());
}
}

0 comments on commit d6638d5

Please sign in to comment.