Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Continuous increase of 'connect' event listeners when manually reconnecting #12

Open
goosy opened this issue Jul 10, 2024 · 1 comment

Comments

@goosy
Copy link

goosy commented Jul 10, 2024

Description

When using S7Endpoint with autoReconnect disabled and manually managing reconnection, the number of 'connect' event listeners keeps increasing. This behavior doesn't occur when using the built-in autoReconnect feature.

Steps to Reproduce

  1. Create an S7Endpoint instance with autoReconnect set to 0.
  2. Add a 'newListener' event handler to log new listeners.
  3. Implement a manual reconnection logic using setInterval.

Expected Behavior

The number of 'connect' event listeners should remain constant, regardless of reconnection attempts.

Actual Behavior

The number of 'connect' event listeners increases with each reconnection attempt, potentially leading to memory leaks or performance issues.

Code to Reproduce

import { S7Endpoint } from '@st-one-io/nodes7';

let plc = new S7Endpoint({ host: '192.168.201.210:102', rack: 0, slot: 1, autoReconnect: 0 });

plc.on('newListener', (eventName, listener) => {
    console.log(`New listener added to ${eventName} event`);
    console.log(`Listener is:`, listener.toString());
    console.log(`Listener count:`, plc.eventNames().map(name => name + ': ' + plc.listenerCount(name)).join(', '));
});

setInterval(async () => {
    if (!plc.isConnected) plc.connect().catch(err => console.log(err));
}, 1000)

Environment

Node.js version: 22.4.0
@st-one-io/nodes7 version: 1.1.0

Additional Observations

  • When autoReconnect is enabled (by setting it to a positive number), this issue does not occur. This suggests that the library's built-in reconnection logic handles listener management more effectively.
  • The increasing listeners appear to be native code functions, as listener.toString() outputs function () { [native code] }.

Questions

  • Is this behavior intended?
  • If not, is there a recommended way to manually manage reconnections without causing listener proliferation?
  • Are there any best practices for managing connections and listeners when using S7Endpoint with manual reconnection logic?

Impact

This issue could potentially lead to memory leaks or degraded performance in long-running applications that require frequent reconnections.

Thank you for your attention to this matter. I appreciate any insights or guidance you can provide.

@mdgarden
Copy link

mdgarden commented Sep 5, 2024

I have same issue and would like to add some additional explanations for the maintainers and others following this issue.

  • The increase in connect event listeners occurs because multiple connect requests are made to the same instance. If you create multiple instances of S7Endpoint and call connect on each instance, the number of event listeners doesn't increase.
  • autoReconnect is not about the number of reconnection attempts, but rather a timer for the timeout to attempt reconnection. Therefore, when autoReconnect is set to 0, S7Endpoint only attempts to connect once for each connect request and doesn't retry further.
    if (this._autoReconnect > 0) {
  • Consequently, if you reproduce the code written by @goosy but change the autoReconnect value to a positive number, you'll see that the event listeners increase just as they do with autoReconnect disabled. As mentioned earlier, the reason for the increase in connect event listeners is that requests are being made to the same instance.
  • As a result, if you want to prevent S7Endpoint's internal reconnection attempts when trying to manually managing reconnect, you can temporarily avoid this issue by setting autoReconnect to 0 and ensuring that only one connect attempt is made per instance.

If there are any inaccuracies in this explanation, please feel free to comment. I hope this issue can be resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants