[plc4x] branch bugfix/close-eventloop-after-channel created (now 5691692)

[jf...@apache.org]

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

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


      at 5691692  Bugfix EventLoop is not closed when Connection is closed and thus can leak.

This branch includes the following new commits:

     new 5691692  Bugfix EventLoop is not closed when Connection is closed and thus can leak.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

[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 {