[Bug]: Modbus - Number of threads repetitively increase at each reconnection.

[frankpuppa]

What happened?

While I was testing the stability of a modbus application based on plc4x, I faced the problem that on successful reconnection attempts the number of live threads keeps increasing continuously. Specifically, there is a pool as such: pool-N-thread-M (probably related to netty) which sits waiting and does not shut down.
The same applies when using the CachedPlcConnectionManager
Connection string: modbus-tcp://127.0.0.1:9081?unit-identifier=1

Here is a sample code with springboot:

 @Scheduled(fixedDelayString = "${scheduling.interval}") 
    public void update() {
        logger.info("Executing update method...")

        try {
            reconnect(false);

            if (plcConnection == null || !plcConnection.isConnected()) {
                logger.info("PLC connection is not alive, exiting...");
                return;
            }

            logger.info("OK");
        } catch (Exception e) {
            logger.error("Error while trying to read ... reconnecting", e);
        }
    }

      protected void reconnect(boolean error) {

        try {
            try {
                if (plcConnection != null && !plcConnection.isConnected()) {
                    logger.info("Reconnecting...");
                    // either with plcConnection.connect() or
                    // connectionManager.getConnection(plcConnectionString); There is a n increment
                    // of threads
                    // plcConnection = connectionManager.getConnection(plcConnectionString);
                    plcConnection.connect();
                    return;
                }
            } catch (Exception e) {
                logger.error("Error: {}", e.getMessage());
                return;
            }

            // Open a new one
            if (plcConnection == null) {
                logger.info("Connecting...");
                plcConnection = connectionManager.getConnection(plcConnectionString);
            }
        } catch (Exception ex) {
            logger.error("Error while trying to reconnect to the plc");
        }
    }

The function runs with a 10 seconds interval, in this time window I simulate the disconnection/reconnection of the cable.
I'm attaching a small video which shows this behavior more clearly.

I understand that this is a corner case but it can lead to a waste of resources which I would like to avoid.

Screencast.2025-08-27.13.38.08.mp4

Thank you.

Version

v0.13.0

Programming Languages

• plc4c
• plc4go
• plc4j
• plc4net
• plc4py

Protocols

• AB-Ethernet
• ADS /AMS
• BACnet/IP
• C-Bus
• CANopen
• EtherNet/IP
• Firmata
• IEC-69870
• KNXnet/IP
• Modbus
• OPC-UA
• Profinet
• S7
• S7-light

[chrisdutz]

Well ... you are actually creating more and more connections with every call to "connect".
For cases like yours we strongly reccomend to have a look at our connection-cache:
https://plc4x.apache.org/plc4x/latest/users/tools/connection-cache.html

[frankpuppa]

Hello Chris, thank you for your reply.

Yes I agree, but old connections should eventually be closed and resources should be releases somehow.

Nevertheless, the code I posted executes this only once:

plcConnection = connectionManager.getConnection(plcConnectionString); 

the rest of the times it simply calls

plcConnection.connect()

which indeed opens a new connections and it works fine, apart from the number of threads that keep increasing.

I should have mentioned that I also tried the connection cache but also in this condition I noticed a strange behavior.
Basically when requesting a plcConnection and the simulator is running, everything works fine.

 PlcConnectionManager connectionManager = CachedPlcConnectionManager.getBuilder().build();
 plcConnection = connectionManager.getConnection(plcConnectionString);

As soon as I disconnect the simulator, even if I reconnect it, the next plcConnection which I get from the cache using:

 plcConnection = connectionManager.getConnection(plcConnectionString);

it is not returned as a valid one. It falls into this condition:

            if (plcConnection == null || !plcConnection.isConnected()) {
                logger.info("PLC connection is not alive, exiting...");
                return;
            }

because plcConnection.isConnected() returns false. Additionally, calling connect to a cached connection returns error: Error connecting leased connection.

To get new alive connections from the connection Manager I have to get a new instance of the CachedPlcConnectionManager but doing so also increases the number of live threads. What am I missing here?
Thank you!

[chrisdutz]

Admittedly we never meant the "connect" method to be called manually ... if you call it, you might re-initialize a new connection based on an old connection's state. I should investigate, if this can happen.

Regardings the ConnectionCache, this would warant for a separate bug report, so we could investigate separately.

[glcj]

Hello,

If you're going to perform a manual reconnection, you should ensure the connection is closed properly by calling the "close" method. This would allow you to kill the tasks associated with the transaction manager, for example.

This may require you to recreate the connection.

It would be interesting to see what tasks remain open with the profiler.

My two cents,

[frankpuppa]

Hello @glcj,
calling close on a disconnected connection raises an exception. Which eventually must be reopened using the

plcConnection = connectionManager.getConnection(plcConnectionString);

I had tried this and under these circumstances, when the new connection opens correctly a new pool is created while the previous sits waiting..
I also tried to manually close the DefaultNettyPlcConnection as such:

EventLoopGroup group = ((DefaultNettyPlcConnection) connection).getChannel().eventLoop().parent();
group.shutdownGracefully();

but it did not help..

[chrisdutz]

So let me understand your test-case a bit more ... you're connecting to a simulator and polling that in regular intervals. As soon as you turn it off (or in a real device, pull the plug or turn it off) things start failing (obviously) but as soon as you make the device available again, it doesn't go back to normal?

[chrisdutz]

But admittedly with respect to maintainability, Netty is a nightmare. That's why I'm already working on a completely Netty-free core for PLC4J ... I do hope the funding, that I applied for, gets granted. Then all of this work will go into PLC4J. If not ... well ... it's go into my commercial offering (Which I hope to have available within a few months)

[frankpuppa]

So let me understand your test-case a bit more ... you're connecting to a simulator and polling that in regular intervals. As soon as you turn it off (or in a real device, pull the plug or turn it off) things start failing (obviously) but as soon as you make the device available again, it doesn't go back to normal?

Hello @chrisdutz
This is exactly the test case. I need to make sure that when the device is available again, everything goes back to normal. At the moment, it does somehow, apart for the number of threads that keep increasing 😄.
While I was investigating the issue I had imagine that it was related to Netty as it seems that it is a common issue with this library (at least from a quick search on the web). Anyway, thank you for letting me know of a possible netty free solution.

[yancyang1989-gif]

你好@glcj， 在断开的连接上调用 close 会引发异常。最终必须使用

plcConnection = connectionManager.getConnection(plcConnectionString);

我曾尝试过这样做，在这种情况下，当新连接正确打开时，会创建一个新池，而前一个连接则处于等待状态。 我还尝试手动关闭 DefaultNettyPlcConnection，如下所示：

EventLoopGroup group = ((DefaultNettyPlcConnection) connection).getChannel().eventLoop().parent();
group.shutdownGracefully();

但它没有帮助。

I think when the connection is accidentally broken, catching the exception by try catch when trying getConnection again, executing CachedPlcConnectionManager.removeCachedConnection in the catch, and doing getConnection again, it is fine

[chrisdutz]

Let's see if Franks PR will solve this problem?

[frankpuppa]

Hello,
Actually I haven't used the CachedPlcConnectionManager and I did not try to call removeCachedConnection. What I did noticed while using the CachedPlcConnectionManager, is that once the channel becomes inactive, the connection retrieved by the pool was always broken even if the device was put back online (no new attempt was made). Perhaps calling removeCachedConnection, may solve the problem of getting a fresh connection but under these circumstances resources may still leak.. Definitely something worth trying..

[QuanticPony]

I think when the connection is accidentally broken, catching the exception by try catch when trying getConnection again, executing CachedPlcConnectionManager.removeCachedConnection in the catch, and doing getConnection again, it is fine

This is what we currently do on the NiFi integration. It gives a fresh connection
But I have not looked into possible leaks there

[michele-tramonti]

I experienced this issue with s7 and opcua too: to mitigate it I made a little change the source code of classes RequestTransactionManager (used by opcua driver and others) and S7ProtocolLogic (used by S7).
Originally in both classes there was a creation of a new ExecutorService (creating threads plc4x-tm-thread-%d and plc4x-app-thread-%d respectively).
It seems to me that every request made with the same connection (i.e. using the cached connection) creates a new object of RequestTransactionManager (in case of non-s7 driver) or S7ProtocolLogic (in case of S7), creating new pool of threads without destroyng the old ones.
So I created a class called SharedExecutor like this:

public class SharedExecutor {
    /**
     * Used by opcua driver and others (but not s7)
     */
    private static final ExecutorService tmExecutor = Executors.newFixedThreadPool(
            10,
            new BasicThreadFactory.Builder()
                    .namingPattern("plc4x-tm-thread-%d")
                    .daemon(true)
                    .priority(Thread.MAX_PRIORITY)
                    .build()
    );

    /**
     * Used only by s7 driver
     */
    private static final ExecutorService appExecutor = Executors.newFixedThreadPool(
            10,
            new BasicThreadFactory.Builder()
                    .namingPattern("plc4x-app-thread-%d")
                    .daemon(true)
                    .priority(Thread.MAX_PRIORITY)
                    .build()
    );

    public static ExecutorService getTmExecutor() {
        return tmExecutor;
    }

    public static ExecutorService getAppExecutor() {
        return appExecutor;
    }
    
}

and using those shared Executors insted of the original ones:

original code in RequestTransactionManager:

final ExecutorService executor = Executors.newFixedThreadPool(4, new BasicThreadFactory.Builder()
                                                    .namingPattern("plc4x-tm-thread-%d")
                                                    .daemon(true)
                                                    .priority(Thread.MAX_PRIORITY)
                                                    .build());

new code:

final ExecutorService executor = SharedExecutor.getTmExecutor();

there are also few little changes to avoid shutting down the shared executor in those classes.

I see it's working but I don't know if, generally speaking, it's a meaningful solution, or only a quick and dirty patch that can cause other issues... that's why I made no pull request...

what do you think?

I'm very interested in a netty-free version of plc4j: is there any news on that?

thank you

[chrisdutz]

Hmmm ... I thought we updated things to close the executor service of discarded connections.

Could you please whip up a PR with your changes so we can have a look? But admittedly I'm always super cautious with singletons. They usually are always the simple way out but always come with huge costs in maintenance. (Like running multiple test-suites in parallel from the same VM (As is usually happens in Maven) ... or if you're running many, many connections on one machine. I would be super careful with them. I think they even break things when using OSGI (I know there are some users/integration modules that rely on the "kar" files we produce)