search

plcpeople / nodeS7

Node.JS library for communication to Siemens S7 PLCs
MIT License
356 stars 120 forks source link

MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 pdu-size listeners added to [S7Endpoint]. Use emitter.setMaxListeners() to increase limit #113

Closed flacohenao closed 4 years ago

flacohenao commented 4 years ago

@gfcittolin Hello!!

I was playing Around with the connection again, testing the new major changes on the Error codes (Even knowing that the work is not fully completed) and... at the same time plating with the connection (connecting - disconnecting) I was reading some variables... and this error happend:

MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 pdu-size listeners added to [S7Endpoint]. Use emitter.setMaxListeners() to increase limit

The conditions were: I was having a PLC with AutoReconnect at 3000 ms.... and what I think is happening is that with the reconnection option, when the link is broken (like I was doing on purpose, connecting-disconnecting) the same instance of the S7Endpoint is not cleaning the listeners when DISCONNECT happens, is that right?

I'm not being able to track further more...

Do you have an Idea of what's happening?

gfcittolin commented 4 years ago

There's indeed a small glitch in the S7ItemGroup, a listener to pdu-size is added at constructor time, but there's no "destructor" so we can remove the listener we added there. We need to add such a destructor method there.

But maybe you can also check your side. Unless you're indeed intentionally creating multiple S7ItemGroup instances for reading multiple groups, this shouldn't happen. When the connection is lost, there's no need to recreate S7ItemGroup. Calling readAllItems() when disconnected will obviously return an error, but once the connection is automatically reestablished by S7Endpoint, it starts reading again.

flacohenao commented 4 years ago

I Think we should definetly add the destructor method there.

When I Disconnect from PLC, I do it in the right way, and for the ReadAllItems() what I do is just Clear the Interval in wich i'm calling the method. So, basicly I was powering-off the PLC and let the auto-reconnect magic do its work and when the connection went back again I just add another interval with a new instance of the S7ItemGroup but in this case, that's intentional because when I detect the disconnect event I just remove the connection from everywhere and I just let the "garbage Collector" collect the unsuded things.

So basicly when I detect the connection again, I just create a new S7ItemGroup.

Maybe I need to change my design to fetch the connection event with the old and only one S7ItemGroup but that would be painfull due to I have connection wich I completly turnoff the reconnection function...

So, let me know if we can resolve that glitch there... or... if I need to change my test design.. (I think it would be better to put a destroyer method there)... or imagine the whole docs we need to create to advise the people the right way to use the S7Group..

what do you think? let me know?

gfcittolin commented 4 years ago

Destructor added. Now you can choose the way you can handle the issue according to the design of your implementation. Even though, I do recommend that you reuse the S7ItemGroup instance. Depending on the architecture, you don't even need to clear the timer, you could just let it running and catch the error.

Besides having to re-add all items, it needs to recompute the optimizations, which is the most computationally intensive code of the library (albeit not that intensive, could have an impact only with a high number of tags)

flacohenao commented 4 years ago

Very nice!!! We're getting out of errors for testing!!! NICE!