dCache / xrootd4j

Implementation of the xrootd data access protocol in Java
Other
3 stars 8 forks source link

xrootd4j: fix byte buffer leak in ZTNCredentialUtils #173

Closed alrossi closed 1 year ago

alrossi commented 1 year ago

Motivation:

When using tokens and ZTN authentication, the following stack trace indicates a mismanaged Netty ByteBuf and presents itself regularly:

30 Jun 2023 10:56:47 (xrootd-1094-fndcatemp2) [] LEAK: ByteBuf.release() was not called before it's garbage-collected. See
https://netty.io/wiki/reference-counted-objects.html for more information.
Recent access records:
Created at:
    io.netty.buffer.PooledByteBufAllocator.newDirectBuffer(PooledByteBufAllocator.java:403)
    io.netty.buffer.AbstractByteBufAllocator.directBuffer(AbstractByteBufAllocator.java:188)
    io.netty.buffer.AbstractByteBufAllocator.buffer(AbstractByteBufAllocator.java:124)
    io.netty.buffer.AbstractByteBuf.readBytes(AbstractByteBuf.java:871)
    org.dcache.xrootd.plugins.authn.ztn.ZTNCredentialUtils.deserialize(ZTNCredentialUtils.java:60)
    org.dcache.xrootd.plugins.authn.ztn.AbstractZTNAuthenticationHandler.authenticate(AbstractZTNAuthenticationHandler.java:63)
    org.dcache.xrootd.core.XrootdAuthenticationHandler.doOnAuthentication(XrootdAuthenticationHandler.java:87)
    org.dcache.xrootd.core.XrootdAuthenticationHandler.getResponse(XrootdAuthenticationHandler.java:101)
    org.dcache.xrootd.core.XrootdRequestHandler.requestReceived(XrootdRequestHandler.java:166)
    org.dcache.xrootd.core.XrootdRequestHandler.channelRead(XrootdRequestHandler.java:115)
...

The issue is that the code calls ByteBuf.readBytes(int) without handling the returned buffer.

This obviously was due to confusion on my part between

ByteBuf.readBytes(int) ByteBuf.readBytes(byte[]) ByteBuf.readByte()

not realizing that the first was not a looped version of the third but actually allocates a new buffer to read the bytes into and returns it.

Modification:

Replace readBytes(2) with readByte() called twice.

Note that this is the only instance in xrootd4j code where readBytes(int) is invoked.

Result:

No more leak and no more stack trace.

Target: master Request: 4.5 Request: 4.4 Request: 4.3 Request: 4.2 Patch: https://rb.dcache.org/r/14010/ Acked-by: Tigran