[plc4x] 01/01: Bugfix EventLoop is not closed when Connection is closed and thus can leak.

[jf...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jfeinauer pushed a commit to branch bugfix/close-eventloop-after-channel
in repository https://gitbox.apache.org/repos/asf/plc4x.git

commit 5691692061b524c98ae38ce5c00ac73e5ec3e132
Author: julian <j....@pragmaticminds.de>
AuthorDate: Mon Aug 24 09:48:26 2020 +0200

    Bugfix EventLoop is not closed when Connection is closed and thus can leak.
---
 .../plc4x/java/spi/connection/ChannelFactory.java    |  5 ++++-
 .../spi/connection/DefaultNettyPlcConnection.java    |  4 ++++
 .../java/spi/connection/NettyChannelFactory.java     | 20 ++++++++++++++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/ChannelFactory.java b/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/ChannelFactory.java
index 82d0b1f..cc96082 100644
--- a/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/ChannelFactory.java
+++ b/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/ChannelFactory.java
@@ -34,6 +34,9 @@ public interface ChannelFactory {
         // Intentionally do Nothing
     }
 
-    //void ping() throws PlcException;
+    default void closeEventLoopForChannel(Channel channel) {
+        // By default do nothing for compatibility
+        // Extending classes should implement their logic here
+    }
 
 }
diff --git a/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/DefaultNettyPlcConnection.java b/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/DefaultNettyPlcConnection.java
index ce43c73..117728b 100644
--- a/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/DefaultNettyPlcConnection.java
+++ b/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/DefaultNettyPlcConnection.java
@@ -132,6 +132,10 @@ public class DefaultNettyPlcConnection extends AbstractPlcConnection implements
         channel.pipeline().fireUserEventTriggered(new CloseConnectionEvent());
         // Close channel
         channel.close().awaitUninterruptibly();
+
+        // Shutdown the Worker Group
+        channelFactory.closeEventLoopForChannel(channel);
+
         channel = null;
         connected = false;
     }
diff --git a/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/NettyChannelFactory.java b/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/NettyChannelFactory.java
index f07b88a..823fcd1 100644
--- a/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/NettyChannelFactory.java
+++ b/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/NettyChannelFactory.java
@@ -30,7 +30,9 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.net.SocketAddress;
+import java.util.Map;
 import java.util.Properties;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * Adapter with sensible defaults for a Netty Based Channel Factory.
@@ -43,6 +45,8 @@ public abstract class NettyChannelFactory implements ChannelFactory {
     private static final Logger logger = LoggerFactory.getLogger(NettyChannelFactory.class);
     private static final int PING_TIMEOUT_MS = 1_000;
 
+    private final Map<Channel, EventLoopGroup> eventLoops = new ConcurrentHashMap<>();
+
     /**
      * TODO should be removed together with the Constructor.
      */
@@ -115,6 +119,9 @@ public abstract class NettyChannelFactory implements ChannelFactory {
 
             final Channel channel = f.channel();
 
+            // Add to Event Loop Group
+            eventLoops.put(channel, workerGroup);
+
             // It seems the embedded channel operates differently.
             // Intentionally using the class name as we don't want to require a
             // hard dependency on the test-channel.
@@ -134,6 +141,19 @@ public abstract class NettyChannelFactory implements ChannelFactory {
         }
     }
 
+    @Override
+    public void closeEventLoopForChannel(Channel channel) {
+        if (eventLoops.containsKey(channel)) {
+            logger.info("Channel is closed, closing worker Group also");
+            EventLoopGroup eventExecutors = eventLoops.get(channel);
+            eventLoops.remove(channel);
+            eventExecutors.shutdownGracefully().awaitUninterruptibly();
+            logger.info("Worker Group was closed successfully!");
+        } else {
+            logger.warn("Trying to remove EventLoop for Channel {} but have none stored", channel);
+        }
+    }
+
     // TODO do we want to keep this like that?
     /*@Override
     public void ping() throws PlcException {