[Bug]: 0.13.0-SNAPSHOT S7 closes TCP connection

[agrazio]

What happened?

Reading eight tags every second from a S7_1200 PLC with S7 with version 0.12.0 it works, but with 0.13.0-SNAPSHOT after a while the connection drops and cannot be restored; it gets stuck in a loop at:

[plc4x-s7ha-thread-1] INFO org.apache.plc4x.java.s7.readwrite.protocol.S7HPlcConnection - Creating primary connection.
[plc4x-s7ha-thread-1] INFO org.apache.plc4x.java.transport.tcp.TcpChannelFactory - Configuring Bootstrap with org.apache.plc4x.java.s7.readwrite.configuration.S7TcpTransportConfiguration@5f4b0cf4
[plc4x-s7ha-thread-1] INFO org.apache.plc4x.java.s7.readwrite.protocol.S7HPlcConnection - Reconnecting primary channel.
[nioEventLoopGroup-427-1] INFO org.apache.plc4x.java.s7.readwrite.protocol.S7HMuxImpl - Unregistered of channel: PRIMARY
[nioEventLoopGroup-427-1] INFO org.apache.plc4x.java.s7.readwrite.protocol.S7HMuxImpl - io.netty.channel.embedded.EmbeddedEventLoop@7da0be76
[plc4x-s7ha-thread-1] INFO org.apache.plc4x.java.s7.readwrite.protocol.S7HPlcConnection - Creating primary connection.
[plc4x-s7ha-thread-1] INFO org.apache.plc4x.java.transport.tcp.TcpChannelFactory - Configuring Bootstrap with org.apache.plc4x.java.s7.readwrite.configuration.S7TcpTransportConfiguration@5f4b0cf4
[plc4x-s7ha-thread-1] INFO org.apache.plc4x.java.s7.readwrite.protocol.S7HPlcConnection - Reconnecting primary channel.
[nioEventLoopGroup-428-1] INFO org.apache.plc4x.java.s7.readwrite.protocol.S7HMuxImpl - Unregistered of channel: PRIMARY
[nioEventLoopGroup-428-1] INFO org.apache.plc4x.java.s7.readwrite.protocol.S7HMuxImpl - io.netty.channel.embedded.EmbeddedEventLoop@7da0be76

Example code:

    public static void main(String[] args) {
            initialize();
            scheduler.scheduleAtFixedRate(() -> executeRead(config.getTags()), 0, 1000, TimeUnit.MILLISECONDS);
    }

    public static void initialize() {
        cachedPlcConnectionManager = CachedPlcConnectionManager.getBuilder().build();
        config = Configuration.getConfiguration();
        scheduler.scheduleAtFixedRate(() -> executeRead(config.getTags()), 0, 10, TimeUnit.MILLISECONDS);
    }

    public static void executeRead(List<Configuration.Tag> tags) {
        try (PlcConnection plcConnection = cachedPlcConnectionManager.getConnection(config.getDeviceAddress())) {
            if (!plcConnection.getMetadata().isReadSupported()) {
                System.out.printf("[%s] Connection does not support reading%n", LocalDateTime.now());
                return;
            }

            PlcReadRequest.Builder readRequestBuilder = plcConnection.readRequestBuilder();
            for (Configuration.Tag tag : tags) {
                readRequestBuilder.addTagAddress(tag.getName(), tag.getAddress());
            }

            PlcReadRequest readRequest = readRequestBuilder.build();
            PlcReadResponse response = readRequest.execute().get();
            processResponse(response);

        } catch (PlcConnectionException e) {
            System.out.printf("[%s] Connection error: %s%n", LocalDateTime.now(), e.getMessage());
        } catch (Exception e) {
            System.out.printf("[%s] Error during PLC read: %s%n", LocalDateTime.now(), e.getMessage());
        }
    }

On failure, it creates up to 40 connections, then closes them, and continues opening and closing connections without establishing a working one.

Version

v0.13.0-SNAPSHOT

Programming Languages

• [x] plc4j
• [ ] plc4go
• [ ] plc4c
• [ ] plc4net

Protocols

• [ ] AB-Ethernet
• [ ] ADS /AMS
• [ ] BACnet/IP
• [ ] CANopen
• [ ] DeltaV
• [ ] DF1
• [ ] EtherNet/IP
• [ ] Firmata
• [ ] KNXnet/IP
• [ ] Modbus
• [ ] OPC-UA
• [x] S7

[DvonHolten]

is it sure be thread-safe? you start a scheduledTask, which runs every 10 milliSecs, then you start another one, which is exceuted every 1000 milliSecs. They may meet somewhen in executeRead(). The method itself looks innocent, but deeper down in readRequest.execute() bad things may happen, when it is not explitly made thread safe.

[chrisdutz]

Well ... if you have one PlcConnection object and you use that with multiple threads this is absolutely going to not work safely. That's what we've created the ConnectionCache for.