From cf43b263218a03308a95d9ac80708c4e0876069e Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Mon, 26 Oct 2020 12:59:20 +0100 Subject: [PATCH] Release message before notify promise (#10726) Motivation: We should preferable always release the message before we notify the promise. Thhis has a few advantages: - Release memory as soon as possible - Listeners observe the "more correct" reference count Modifications: Release message before fail the promises Result: Faster releasing of resources. This came up in https://github.com/netty/netty/issues/10723 --- .../io/netty/channel/AbstractChannel.java | 25 ++++++++++++------- .../channel/DefaultChannelHandlerContext.java | 4 +-- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/transport/src/main/java/io/netty/channel/AbstractChannel.java b/transport/src/main/java/io/netty/channel/AbstractChannel.java index ecac1a53a8..b77ae825a1 100644 --- a/transport/src/main/java/io/netty/channel/AbstractChannel.java +++ b/transport/src/main/java/io/netty/channel/AbstractChannel.java @@ -824,13 +824,17 @@ public abstract class AbstractChannel extends DefaultAttributeMap implements Cha ChannelOutboundBuffer outboundBuffer = this.outboundBuffer; if (outboundBuffer == null) { - // If the outboundBuffer is null we know the channel was closed and so - // need to fail the future right away. If it is not null the handling of the rest - // will be done in flush0() - // See https://github.com/netty/netty/issues/2362 - safeSetFailure(promise, newClosedChannelException(initialCloseCause, "write(Object, ChannelPromise)")); - // release message now to prevent resource-leak - ReferenceCountUtil.release(msg); + try { + // release message now to prevent resource-leak + ReferenceCountUtil.release(msg); + } finally { + // If the outboundBuffer is null we know the channel was closed and so + // need to fail the future right away. If it is not null the handling of the rest + // will be done in flush0() + // See https://github.com/netty/netty/issues/2362 + safeSetFailure(promise, + newClosedChannelException(initialCloseCause, "write(Object, ChannelPromise)")); + } return; } @@ -845,8 +849,11 @@ public abstract class AbstractChannel extends DefaultAttributeMap implements Cha size = 0; } } catch (Throwable t) { - safeSetFailure(promise, t); - ReferenceCountUtil.release(msg); + try { + ReferenceCountUtil.release(msg); + } finally { + safeSetFailure(promise, t); + } return; } diff --git a/transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java b/transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java index 0e094d7034..39d3caf18f 100644 --- a/transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java +++ b/transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java @@ -973,11 +973,11 @@ final class DefaultChannelHandlerContext implements ChannelHandlerContext, Resou return true; } catch (Throwable cause) { try { - promise.setFailure(cause); - } finally { if (msg != null) { ReferenceCountUtil.release(msg); } + } finally { + promise.setFailure(cause); } return false; }