From f5185ed73b347ba684eef2348622030b5cbe99cf Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
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 ++++++++++++-------
 .../AbstractChannelHandlerContext.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 fdbc46ad20..61fec71d92 100644
--- a/transport/src/main/java/io/netty/channel/AbstractChannel.java
+++ b/transport/src/main/java/io/netty/channel/AbstractChannel.java
@@ -859,13 +859,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;
             }
 
@@ -877,8 +881,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/AbstractChannelHandlerContext.java b/transport/src/main/java/io/netty/channel/AbstractChannelHandlerContext.java
index 22fa05a2c0..348ba3a558 100644
--- a/transport/src/main/java/io/netty/channel/AbstractChannelHandlerContext.java
+++ b/transport/src/main/java/io/netty/channel/AbstractChannelHandlerContext.java
@@ -991,11 +991,11 @@ abstract class AbstractChannelHandlerContext implements ChannelHandlerContext, R
             return true;
         } catch (Throwable cause) {
             try {
-                promise.setFailure(cause);
-            } finally {
                 if (msg != null) {
                     ReferenceCountUtil.release(msg);
                 }
+            } finally {
+                promise.setFailure(cause);
             }
             return false;
         }