From 171fbf3b41e64a74a7c4522592b291e5a45f1063 Mon Sep 17 00:00:00 2001 From: nmittler Date: Thu, 20 Nov 2014 15:09:43 -0800 Subject: [PATCH] Race condition with completion of Channel closeFuture and unregistration. Motivation: AbstractChannel.close() completes the closeFuture and then proceeds to schedule a task to fireChannelInactive and unregister the channel. If the user shuts down the EventLoop as a result of the closeFuture completing this can result in a RejectedExecutionException when the unregister task is scheduled. This is mostly noise, but it does somewhat pollute the logs. Modifications: Changed AbstractChannel.close() to not complete the channelFuture and promise until after the unregistration task is scheduled. Result: Noisy error log should no longer be an issue. --- .../java/io/netty/channel/AbstractChannel.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/transport/src/main/java/io/netty/channel/AbstractChannel.java b/transport/src/main/java/io/netty/channel/AbstractChannel.java index ad2deae6c7..04a7c416bb 100644 --- a/transport/src/main/java/io/netty/channel/AbstractChannel.java +++ b/transport/src/main/java/io/netty/channel/AbstractChannel.java @@ -617,13 +617,11 @@ public abstract class AbstractChannel extends DefaultAttributeMap implements Cha ChannelOutboundBuffer outboundBuffer = this.outboundBuffer; this.outboundBuffer = null; // Disallow adding any messages and flushes to outboundBuffer. + Throwable error = null; try { doClose(); - closeFuture.setClosed(); - safeSetSuccess(promise); } catch (Throwable t) { - closeFuture.setClosed(); - safeSetFailure(promise, t); + error = t; } // Fail all the queued messages @@ -647,6 +645,14 @@ public abstract class AbstractChannel extends DefaultAttributeMap implements Cha } }); } + + // Now complete the closeFuture and promise. + closeFuture.setClosed(); + if (error != null) { + safeSetFailure(promise, error); + } else { + safeSetSuccess(promise); + } } }