You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by jf...@apache.org on 2020/02/13 23:18:19 UTC

[plc4x] branch rel/0.6 updated: [S7] Refactoring / Leak Fixing.

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

jfeinauer pushed a commit to branch rel/0.6
in repository https://gitbox.apache.org/repos/asf/plc4x.git


The following commit(s) were added to refs/heads/rel/0.6 by this push:
     new 04ce149  [S7] Refactoring / Leak Fixing.
04ce149 is described below

commit 04ce1493bc13e0114c27b8b680e0de6741540b36
Author: Julian Feinauer <j....@pragmaticminds.de>
AuthorDate: Fri Feb 14 00:18:02 2020 +0100

    [S7] Refactoring / Leak Fixing.
---
 .../plc4x/java/s7/connection/S7PlcConnection.java     | 19 +++++++++++--------
 .../java/tcp/connection/TcpSocketChannelFactory.java  | 16 ++++++++++++----
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java b/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java
index 0de6483..b528f1d 100644
--- a/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java
+++ b/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java
@@ -84,7 +84,7 @@ public class S7PlcConnection extends NettyPlcConnection implements PlcReader, Pl
 
     // Fetch values from configuration
     private static final Configuration CONF = new SystemConfiguration();
-    private static final long CLOSE_DEVICE_TIMEOUT_MS = CONF.getLong("plc4x.s7connection.close.device,timeout", 1_000);
+    private static final long CLOSE_DEVICE_TIMEOUT_MS = CONF.getLong("plc4x.s7connection.close.device,timeout", 10_000);
 
     private static final Logger logger = LoggerFactory.getLogger(S7PlcConnection.class);
 
@@ -234,6 +234,7 @@ public class S7PlcConnection extends NettyPlcConnection implements PlcReader, Pl
 
     @Override
     public void close() throws PlcConnectionException {
+        logger.debug("S7 Connection's close method is triggered");
         if ((channel != null) && channel.isOpen()) {
             // Send the PLC a message that the connection is being closed.
             DisconnectRequestTpdu disconnectRequest = new DisconnectRequestTpdu(
@@ -247,10 +248,12 @@ public class S7PlcConnection extends NettyPlcConnection implements PlcReader, Pl
                 (ChannelFutureListener) future -> disconnectFuture.complete(null));
 
             // Send the disconnect request.
+            logger.trace("Sending disconnect request to PLC");
             channel.writeAndFlush(disconnectRequest);
             // Wait for the configured time for the remote to close the session.
             try {
                 disconnectFuture.get(CLOSE_DEVICE_TIMEOUT_MS, TimeUnit.MILLISECONDS);
+                logger.trace("Got disconnect response from PLC, can close channel now.");
             }
             // If the remote didn't close the connection within the given time-frame, we have to take
             // care of closing the connection.
@@ -261,13 +264,13 @@ public class S7PlcConnection extends NettyPlcConnection implements PlcReader, Pl
                 Thread.currentThread().interrupt();
             } catch (ExecutionException e) {
                 throw new PlcConnectionException(e);
-            }
-
-            // Do some additional cleanup operations ...
-            // In normal operation, the channels event loop has a parent, however when running with
-            // the embedded channel for unit tests, parent is null.
-            if (channel.eventLoop().parent() != null) {
-                channel.eventLoop().parent().shutdownGracefully();
+            } finally {
+                // Do some additional cleanup operations ...
+                // In normal operation, the channels event loop has a parent, however when running with
+                // the embedded channel for unit tests, parent is null.
+                if (channel.eventLoop().parent() != null) {
+                    channel.eventLoop().parent().shutdownGracefully();
+                }
             }
         }
         super.close();
diff --git a/plc4j/protocols/driver-bases/tcp/src/main/java/org/apache/plc4x/java/tcp/connection/TcpSocketChannelFactory.java b/plc4j/protocols/driver-bases/tcp/src/main/java/org/apache/plc4x/java/tcp/connection/TcpSocketChannelFactory.java
index 0ab4f15..8f9f55b 100644
--- a/plc4j/protocols/driver-bases/tcp/src/main/java/org/apache/plc4x/java/tcp/connection/TcpSocketChannelFactory.java
+++ b/plc4j/protocols/driver-bases/tcp/src/main/java/org/apache/plc4x/java/tcp/connection/TcpSocketChannelFactory.java
@@ -66,16 +66,24 @@ public class TcpSocketChannelFactory implements ChannelFactory {
         // bootstrap.option(ChannelOption.CONNECT_TIMEOUT_MILLIS, 1000);
         bootstrap.handler(channelHandler);
         // Start the client.
+        ChannelFuture f = null;
         try {
             logger.trace("Starting connection attempt on tcp layer to {}:{}", address.getHostAddress(), port);
-            final ChannelFuture f = bootstrap.connect(address, port);
+            f = bootstrap.connect(address, port)
+                .addListener(event -> {
+                    logger.trace("Connection process finished, success is {}", event.isSuccess());
+                });
             // Wait for sync
-
-            f.sync();
             // Wait till the session is finished initializing.
+            f.sync();
+            logger.trace("Sync completed, connection established on tcp layer");
+
             return f.channel();
-        } catch (Exception e) {
+        } catch (Throwable e) {
             // Shutdown worker group here and wait for it
+            if (f != null) {
+                f.channel().closeFuture().awaitUninterruptibly();
+            }
             logger.info("Unable to connect, shutting down worker thread.");
             workerGroup.shutdownGracefully().awaitUninterruptibly();
             logger.debug("Worker Group is shutdown successfully.");