You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@plc4x.apache.org by Stephen Snow <s4...@gmail.com> on 2022/04/27 12:06:11 UTC

Re: [jira] [Commented] (PLC4X-339) modbus connection causes memory leak

On Wed, 2022-04-27 at 07:30 +0000, Christofer Dutz (Jira) wrote:
> 
>     [
> https://issues.apache.org/jira/browse/PLC4X-339?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17528601#comment-17528601
>  ]
> 
> Christofer Dutz commented on PLC4X-339:
> ---------------------------------------
> 
> I guess in the end er really should get rid of Netty ... it just adds
> loads of complexity and problems like this ... plc4go and plc4c also
> work fine without any Netty.
> 
Go and C don't use Java libraries for networking protocols like TCP or
UDP I guess, but isn't Netty sort of integral to the communication
stack right now? Just asking is all. I mean it is pretty powerful for
setting up TCP and UDP connections, and serial AFAICT.
> > modbus connection causes memory leak
> > ------------------------------------
> > 
> >                 Key: PLC4X-339
> >                 URL:
> > https://issues.apache.org/jira/browse/PLC4X-339
> >             Project: Apache PLC4X
> >          Issue Type: Bug
> >          Components: Driver-Modbus
> >    Affects Versions: 0.9.0
> >         Environment: Windows 10; jdk1.8 32bit
> >            Reporter: liangjian
> >            Priority: Major
> > 
> > Note: The version is 0.9.1.
> > 1. The document
> > (here)[https://plc4x.apache.org/users/protocols/modbus.html] seems
> > wrong. It tells me the connection string format is: "modbus-
> > tcp:tcp://127.0.0.1:502", but my test code throws exception saying
> > no driver until I change it to "modbus://127.0.0.1".
> > 2. My test code writes (or reads) some value via modbus tcp every 1
> > second, using short tcp connection (open and close every time).
From the Netty documentation

"Please note that, close() also might not close the connection
immediately, and it returns a ChannelFuture."

How is the future being handled? Close doesn't always close immediately
according to what it says in the netty documentation at
https://netty.io/wiki/user-guide-for-5.x.html


> >  The memory increases every second until out-of-memory. In process
> > monitor I find the thread count  increases 1 every time the
> > connection opens, but never releases, althought I consider the
> > `try-block` shall closes it automatically. In fact, I see it calls
> > `connection.close` after the `try-block`.
Does connection.close return success? Or anything? Possibly it needs to
reflect that the close operation sometimes doesn't happen immediately
upon request. Again, just asking here since this is an asynchronous
operation, and the thread would stay open until the future completed I
would think.
> > Test code:
> > ```java
> >         while (true) {
> >             String connectionString = "modbus://127.0.0.1";
> >             try (PlcConnection plcConnection = new
> > PlcDriverManager().getConnection(connectionString)) {
> >                 PlcWriteRequest.Builder builder =
> > plcConnection.writeRequestBuilder();
> >                 builder.addItem("value-1", "holding-
> > register:1:DINT", 30000);
> >                 builder.addItem("value-2", "holding-
> > register:3:REAL", 3.14);
> >                 PlcWriteRequest writeRequest = builder.build();
> >                 PlcWriteResponse response =
> > writeRequest.execute().get();
> >                 
> >                 for (String fieldName : response.getFieldNames()) {
> >                     if(response.getResponseCode(fieldName) ==
> > PlcResponseCode.OK) {
> >                         System.out.println("Value[" + fieldName +
> > "]: updated");
> >                     }
> >                     // Something went wrong, to output an error
> > message instead.
> >                     else {
> >                         System.out.println("Error[" + fieldName +
> > "]: " + response.getResponseCode(fieldName).name());
> >                     }
> >                 }
> >                 System.out.println("done");
> >             }
> >             Thread.sleep(1000);
> >         }
> > ```
> > It shows the connection is not cleaned properly. 
> > I see it's more efficient to do this using long tcp connection
> > (reuse 1 connection or use a pool). But short tcp connection should
> > also work.
Possibly the "long tcp connection" (is this in time/duration?) success
is an indicator that the future promise has completed?
> >  

> 
> 
> 
> --
> This message was sent by Atlassian Jira
> (v8.20.7#820007)

Regards,
Stephen

RE: [jira] [Commented] (PLC4X-339) modbus connection causes memory leak

Posted by Christofer Dutz <ch...@c-ware.de>.
Hi Stephen,

I think our usage of Netty in PLC4J is historically ... Initially I hand wrote drivers and modelled them as layers that I could combine. To weave everything together, I started with Apache Mina and quickly switched to Netty. 

The way our new generated drivers are being built however we no longer have these layers. So effectively we're using one layer and having Netty manage that ... I'd love to get rid of Netty, however this would mean a rewrite of the Java SPI, which currently nobody would be willing to do or finance.

The communication endpoints TCP and UDP are quite simple to use in Netty, however it's internally sort of a mess ... I had to jump though loads of hoops to get the virtual "test-channel" working for our unit and integration-tests. 

I don't think we would need this complex internal setup for things as simple as our drivers. We see the other drivers work fine without and we would have the advantage of having full control over all parts of the SPI. Which again would be a huge step towards generating more and more parts of the drivers.


Chris


-----Original Message-----
From: Stephen Snow <s4...@gmail.com> 
Sent: Mittwoch, 27. April 2022 14:06
To: dev@plc4x.apache.org; issues@plc4x.apache.org
Subject: Re: [jira] [Commented] (PLC4X-339) modbus connection causes memory leak

On Wed, 2022-04-27 at 07:30 +0000, Christofer Dutz (Jira) wrote:
> 
>     [
> https://issues.apache.org/jira/browse/PLC4X-339?page=com.atlassian.jir
> a.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17528
> 601#comment-17528601
>  ]
> 
> Christofer Dutz commented on PLC4X-339:
> ---------------------------------------
> 
> I guess in the end er really should get rid of Netty ... it just adds 
> loads of complexity and problems like this ... plc4go and plc4c also 
> work fine without any Netty.
> 
Go and C don't use Java libraries for networking protocols like TCP or UDP I guess, but isn't Netty sort of integral to the communication stack right now? Just asking is all. I mean it is pretty powerful for setting up TCP and UDP connections, and serial AFAICT.
> > modbus connection causes memory leak
> > ------------------------------------
> > 
> >                 Key: PLC4X-339
> >                 URL:
> > https://issues.apache.org/jira/browse/PLC4X-339
> >             Project: Apache PLC4X
> >          Issue Type: Bug
> >          Components: Driver-Modbus
> >    Affects Versions: 0.9.0
> >         Environment: Windows 10; jdk1.8 32bit
> >            Reporter: liangjian
> >            Priority: Major
> > 
> > Note: The version is 0.9.1.
> > 1. The document
> > (here)[https://plc4x.apache.org/users/protocols/modbus.html] seems 
> > wrong. It tells me the connection string format is: "modbus- 
> > tcp:tcp://127.0.0.1:502", but my test code throws exception saying 
> > no driver until I change it to "modbus://127.0.0.1".
> > 2. My test code writes (or reads) some value via modbus tcp every 1 
> > second, using short tcp connection (open and close every time).
From the Netty documentation

"Please note that, close() also might not close the connection immediately, and it returns a ChannelFuture."

How is the future being handled? Close doesn't always close immediately according to what it says in the netty documentation at https://netty.io/wiki/user-guide-for-5.x.html


> >  The memory increases every second until out-of-memory. In process 
> > monitor I find the thread count  increases 1 every time the 
> > connection opens, but never releases, althought I consider the 
> > `try-block` shall closes it automatically. In fact, I see it calls 
> > `connection.close` after the `try-block`.
Does connection.close return success? Or anything? Possibly it needs to reflect that the close operation sometimes doesn't happen immediately upon request. Again, just asking here since this is an asynchronous operation, and the thread would stay open until the future completed I would think.
> > Test code:
> > ```java
> >         while (true) {
> >             String connectionString = "modbus://127.0.0.1";
> >             try (PlcConnection plcConnection = new
> > PlcDriverManager().getConnection(connectionString)) {
> >                 PlcWriteRequest.Builder builder = 
> > plcConnection.writeRequestBuilder();
> >                 builder.addItem("value-1", "holding- 
> > register:1:DINT", 30000);
> >                 builder.addItem("value-2", "holding- 
> > register:3:REAL", 3.14);
> >                 PlcWriteRequest writeRequest = builder.build();
> >                 PlcWriteResponse response = 
> > writeRequest.execute().get();
> >                 
> >                 for (String fieldName : response.getFieldNames()) {
> >                     if(response.getResponseCode(fieldName) ==
> > PlcResponseCode.OK) {
> >                         System.out.println("Value[" + fieldName +
> > "]: updated");
> >                     }
> >                     // Something went wrong, to output an error 
> > message instead.
> >                     else {
> >                         System.out.println("Error[" + fieldName +
> > "]: " + response.getResponseCode(fieldName).name());
> >                     }
> >                 }
> >                 System.out.println("done");
> >             }
> >             Thread.sleep(1000);
> >         }
> > ```
> > It shows the connection is not cleaned properly. 
> > I see it's more efficient to do this using long tcp connection 
> > (reuse 1 connection or use a pool). But short tcp connection should 
> > also work.
Possibly the "long tcp connection" (is this in time/duration?) success is an indicator that the future promise has completed?
> >  

> 
> 
> 
> --
> This message was sent by Atlassian Jira
> (v8.20.7#820007)

Regards,
Stephen