thingsboard / tbmq

Open-source, scalable, and fault-tolerant MQTT broker able to handle 4M+ concurrent client connections, supporting at least 3M messages per second throughput per single cluster node with low latency delivery. The cluster mode supports more than 100M concurrently connected clients.
https://thingsboard.io/products/mqtt-broker/
Apache License 2.0
570 stars 46 forks source link

[Bug] Is there a memory leak in WsByteBufEncoder? #96

Closed GeXin666 closed 7 months ago

GeXin666 commented 7 months ago

`public class WsByteBufEncoder extends MessageToMessageEncoder {

@Override
protected void encode(ChannelHandlerContext ctx, ByteBuf msg, List<Object> out) throws Exception {
    final BinaryWebSocketFrame binaryWebSocketFrame = new BinaryWebSocketFrame(msg);
    ReferenceCountUtil.retain(binaryWebSocketFrame);
    out.add(binaryWebSocketFrame);
}

}`

version:1.2.2-SNAPSHOT ReferenceCountUtil.retain(binaryWebSocketFrame);
The counter should not be incremented by one here?

dmytro-landiak commented 7 months ago

Hi @GeXin666!

Thank you for the issue you raised. We appreciate your interest in this regard. There is no known issue regarding memory leaks when using WebSockets over MQTT.

The ReferenceCountUtil.retain(binaryWebSocketFrame); call in WsByteBufEncoder is necessary because when a ByteBuf is sent through the pipeline, it will be released (reference count decremented) once it is written out. Since BinaryWebSocketFrame wraps this ByteBuf, you must retain it to prevent premature release as it passes through the pipeline and ensures it isn't deallocated before all handlers have finished processing it. This balances out the release call that will happen later, maintaining the correct reference count.

To check if a memory leak is present or not, you can set the following parameter:

image

The link: https://thingsboard.io/docs/mqtt-broker/install/config/#mqtt-listeners-parameters

leak_detector_level: "${WS_NETTY_LEAK_DETECTOR_LVL:PARANOID}"

After you set it, you will see the errors in the logs if a memory leak is present. Normally, this is not the case.

For more details, I would recommend the following article: https://netty.io/wiki/reference-counted-objects.html

GeXin666 commented 7 months ago

all right! Thank you for your answer,I forgot that MessageToMessageEncoder did a release of bytebuf onces。so you need set the counter +1

out = CodecOutputList.newInstance(); @SuppressWarnings("unchecked") I cast = (I) msg; try { encode(ctx, cast, out); } catch (Throwable th) { ReferenceCountUtil.safeRelease(cast); PlatformDependent.throwException(th); } ReferenceCountUtil.release(cast);

            if (out.isEmpty()) {
                throw new EncoderException(
                        StringUtil.simpleClassName(this) + " must produce at least one message.");

}