-
-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Leak detection combined with composite buffers results in incorrectly handled writerIndex #8152
Description
This is a nasty bug, which only manifests with leak detection, so would be very confusing in the wild. It was discovered in a test suite which validated with a range of buffer types and resulted in a flaky test. Fortunately composite buffers are not in common use, and leak detection is not permanently on, so the bug should not effect most people most of the time. This also made it harder to hunt down.
In some places the writerIndex field of AbstractByteBuf is manipulated directly under the assumption that subclasses of AbstractByteBuf are safe to manipulate, while subclasses of WrappedByteBuf are to be unwrapped until, hopefully, an AbstractByteBuf will emerge.
E.g.:
public static int writeAscii(ByteBuf buf, CharSequence seq) {
// ASCII uses 1 byte per char
final int len = seq.length();
if (seq instanceof AsciiString) {
AsciiString asciiString = (AsciiString) seq;
buf.writeBytes(asciiString.array(), asciiString.arrayOffset(), len);
} else {
for (;;) {
if (buf instanceof AbstractByteBuf) {
AbstractByteBuf byteBuf = (AbstractByteBuf) buf;
byteBuf.ensureWritable(len);
int written = writeAscii(byteBuf, byteBuf.writerIndex, seq, len);
byteBuf.writerIndex += written;
return written;
} else if (buf instanceof WrappedByteBuf) {
// Unwrap as the wrapped buffer may be an AbstractByteBuf and so we can use fast-path.
buf = buf.unwrap();
} else {
buf.writeBytes(seq.toString().getBytes(CharsetUtil.US_ASCII));
}
}
}
return len;
}
The assumption breaks with WrappedCompositeByteBuf as it is a subclass of AbstractByteBuf via CompositeByteBuf, but should have perhaps been a subclass of WrappedByteBuf. The code ends up writing the data to the 0 offset, overwriting whatever it finds there.
I've hit the resulting bug on writeUtf and can see it would also manifest for writeAscii, and perhaps elsewhere. Leak detection for composite buffers extends WrappedCompositeByteBuf. The bug can be fixed by using the accessor methods instead of the field.
Steps to reproduce
Run a writeAscii/Utf test with paranoid leak detection to reproduce.
Minimal yet complete reproducer code (or URL to code)
Netty version
4.1, perhaps earlier too, I did not check.