JulianFeinauer closed pull request #10: Fixed NPE in S7PlcConnection#close.
URL: https://github.com/apache/incubator-plc4x/pull/10
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/plc4j/protocols/s7/pom.xml b/plc4j/protocols/s7/pom.xml
index 75dec7147..cd95354e2 100644
--- a/plc4j/protocols/s7/pom.xml
+++ b/plc4j/protocols/s7/pom.xml
@@ -101,6 +101,10 @@
<version>0.0.1-SNAPSHOT</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-configuration2</artifactId>
+ </dependency>
</dependencies>
</project>
\ No newline at end of file
diff --git
a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java
b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java
index fd8c98dad..eec4384f3 100644
---
a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java
+++
b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java
@@ -19,6 +19,8 @@ Licensed to the Apache Software Foundation (ASF) under one
package org.apache.plc4x.java.s7.connection;
import io.netty.channel.*;
+import org.apache.commons.configuration2.Configuration;
+import org.apache.commons.configuration2.SystemConfiguration;
import org.apache.commons.lang3.StringUtils;
import org.apache.plc4x.java.api.connection.PlcReader;
import org.apache.plc4x.java.api.connection.PlcWriter;
@@ -48,8 +50,11 @@ Licensed to the Apache Software Foundation (ASF) under one
import org.slf4j.LoggerFactory;
import java.net.InetAddress;
+import java.time.temporal.ChronoUnit;
import java.util.Collections;
import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -75,6 +80,10 @@ Licensed to the Apache Software Foundation (ASF) under one
private static final int ISO_ON_TCP_PORT = 102;
+ // 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 Pattern S7_DATABLOCK_ADDRESS_PATTERN =
Pattern.compile("^DATA_BLOCKS/(?<blockNumber>\\d{1,4})/(?<byteOffset>\\d{1,4})");
private static final Pattern S7_ADDRESS_PATTERN =
@@ -193,30 +202,6 @@ public int getParamMaxAmqCallee() {
return paramMaxAmqCallee;
}
- @Override
- public void close() {
- if ((channel != null) && channel.isOpen()) {
- // Send the PLC a message that the connection is being closed.
- DisconnectRequestTpdu disconnectRequest = new
DisconnectRequestTpdu(
- (short) 0x0000, (short) 0x000F, DisconnectReason.NORMAL,
Collections.emptyList(),
- null);
- channel.writeAndFlush(disconnectRequest).awaitUninterruptibly();
-
- // In case of an ISO TP Class 0 connection, the remote is usually
expected to actively
- // close the connection. We are simplifying things by doing it
ourselves immediately.
- // TODO: It would probably be more spec-conform to wait for the
remote to disconnect and to only do it actively if it doesn't happen within a
certain time. But this solution is good enough for now.
- channel.close();
-
- // 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();
- }
-
-
@Override
public Address parseAddress(String addressString) throws PlcException {
Matcher datablockAddressMatcher =
S7_DATABLOCK_ADDRESS_PATTERN.matcher(addressString);
@@ -257,4 +242,45 @@ public Address parseAddress(String addressString) throws
PlcException {
return writeFuture;
}
+ /**
+ * Closes the S7 connection.
+ * First, a disconnect request is send to the PLC and after that, the
Session and the channel are closed and
+ * invalidated.
+ *
+ * Waits if the PLC disconnects for CLOSE_DEVICE_TIMEOUT_MS and closes the
connection itself, otherwise.
+ */
+ @Override
+ public void close() {
+ if ((channel != null) && channel.isOpen()) {
+ // Send the PLC a message that the connection is being closed.
+ DisconnectRequestTpdu disconnectRequest = new
DisconnectRequestTpdu(
+ (short) 0x0000, (short) 0x000F, DisconnectReason.NORMAL,
Collections.emptyList(),
+ null);
+ channel.writeAndFlush(disconnectRequest);
+
+ // In case of an ISO TP Class 0 connection, the remote is usually
expected to actively
+ // CountDownLatch has to be used instead closeFuture().await() due
to netty permitting calling await()
+ // on Executors that server as event loop
+ CountDownLatch closeLatch = new CountDownLatch(1);
+ channel.closeFuture().addListener(listener ->
closeLatch.countDown());
+ try {
+ closeLatch.await(CLOSE_DEVICE_TIMEOUT_MS,
TimeUnit.MILLISECONDS);
+ } catch (InterruptedException e) {
+ logger.warn("Connection was not closed by PLC, has to be
closed from driver side now.", e);
+ Thread.currentThread().interrupt();
+ }
+ // If the remote did not close the connection above, then we do it
ourselves.
+ if (channel.isOpen()) {
+ channel.close();
+ }
+
+ // 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();
+ }
+
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
