[#4449] Remove registered events from eventloop before close

Motivation:

We need to remove all registered events for a Channel from the EventLoop before doing the actual close to ensure we not produce a cpu spin when the actual close operation is delayed or executed outside of the EventLoop.

Modifications:

Deregister for events for NIO and EPOLL socket implementations when SO_LINGER is used.

Result:

No more cpu spin.

transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollSocketChannel.java

@@ -160,7 +160,7 @@ public final class EpollSocketChannel extends AbstractEpollStreamChannel impleme
 
     @Override
     public ChannelFuture shutdownOutput(final ChannelPromise promise) {
-        Executor closeExecutor = ((EpollSocketChannelUnsafe) unsafe()).closeExecutor();
+        Executor closeExecutor = ((EpollSocketChannelUnsafe) unsafe()).prepareToClose();
         if (closeExecutor != null) {
             closeExecutor.execute(new OneTimeTask() {
                 @Override
@@ -237,11 +237,16 @@ public final class EpollSocketChannel extends AbstractEpollStreamChannel impleme
 
     private final class EpollSocketChannelUnsafe extends EpollStreamUnsafe {
         @Override
-        protected Executor closeExecutor() {
+        protected Executor prepareToClose() {
             try {
                 // Check isOpen() first as otherwise it will throw a RuntimeException
                 // when call getSoLinger() as the fd is not valid anymore.
                 if (isOpen() && config().getSoLinger() > 0) {
+                    // We need to cancel this key of the channel so we may not end up in a eventloop spin
+                    // because we try to read or write until the actual close happens which may be later due
+                    // SO_LINGER handling.
+                    // See https://github.com/netty/netty/issues/4449
+                    ((EpollEventLoop) eventLoop()).remove(EpollSocketChannel.this);
                     return GlobalEventExecutor.INSTANCE;
                 }
             } catch (Throwable ignore) {

transport/src/main/java/io/netty/channel/AbstractChannel.java

@@ -615,7 +615,7 @@ public abstract class AbstractChannel extends DefaultAttributeMap implements Cha
 
             final boolean wasActive = isActive();
             this.outboundBuffer = null; // Disallow adding any messages and flushes to outboundBuffer.
-            Executor closeExecutor = closeExecutor();
+            Executor closeExecutor = prepareToClose();
             if (closeExecutor != null) {
                 closeExecutor.execute(new OneTimeTask() {
                     @Override
@@ -916,11 +916,12 @@ public abstract class AbstractChannel extends DefaultAttributeMap implements Cha
         }
 
         /**
-         * @return {@link Executor} to execute {@link #doClose()} or {@code null} if it should be done in the
-         * {@link EventLoop}.
-         +
+         * Prepares to close the {@link Channel}. If this method returns an {@link Executor}, the
+         * caller must call the {@link Executor#execute(Runnable)} method with a task that calls
+         * {@link #doClose()} on the returned {@link Executor}. If this method returns {@code null},
+         * {@link #doClose()} must be called from the caller thread. (i.e. {@link EventLoop})
          */
-        protected Executor closeExecutor() {
+        protected Executor prepareToClose() {
             return null;
         }
     }

transport/src/main/java/io/netty/channel/socket/nio/NioSocketChannel.java

@@ -36,7 +36,6 @@ import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.net.Socket;
 import java.net.SocketAddress;
-import java.net.SocketException;
 import java.nio.ByteBuffer;
 import java.nio.channels.SelectionKey;
 import java.nio.channels.SocketChannel;
@@ -152,7 +151,7 @@ public class NioSocketChannel extends AbstractNioByteChannel implements io.netty
 
     @Override
     public ChannelFuture shutdownOutput(final ChannelPromise promise) {
-        Executor closeExecutor = ((NioSocketChannelUnsafe) unsafe()).closeExecutor();
+        Executor closeExecutor = ((NioSocketChannelUnsafe) unsafe()).prepareToClose();
         if (closeExecutor != null) {
             closeExecutor.execute(new OneTimeTask() {
                 @Override
@@ -336,9 +335,14 @@ public class NioSocketChannel extends AbstractNioByteChannel implements io.netty
 
     private final class NioSocketChannelUnsafe extends NioByteUnsafe {
         @Override
-        protected Executor closeExecutor() {
+        protected Executor prepareToClose() {
             try {
                 if (javaChannel().isOpen() && config().getSoLinger() > 0) {
+                    // We need to cancel this key of the channel so we may not end up in a eventloop spin
+                    // because we try to read or write until the actual close happens which may be later due
+                    // SO_LINGER handling.
+                    // See https://github.com/netty/netty/issues/4449
+                    doDeregister();
                     return GlobalEventExecutor.INSTANCE;
                 }
             } catch (Throwable ignore) {