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

Node process hangs before exiting #99

Closed
bvaughn opened this issue Apr 11, 2024 · 12 comments
Closed

Node process hangs before exiting #99

bvaughn opened this issue Apr 11, 2024 · 12 comments

Comments

@bvaughn
Copy link

bvaughn commented Apr 11, 2024

Describe the bug
In the event of no Internet connection, the js-sdk-common module seems to prevent process.exit() from completing for several seconds (up to ~15s in my own testing). This seems to be related to pending async calls (and removing this module from the NodeJS app I have been developing fixes this problem completely).

To reproduce
Run the following code in NodeJS while disconnected from the Internet:

import { initialize } from "launchdarkly-node-client-sdk";

async function repro() {
  let client;
  try {
    console.log("Initializing LD client ...");
    client = initialize("...key...", {
      anonymous: true,
    });
  } catch (error) {
    console.log("ERROR: LD client initialization failed");
  }

  try {
    console.log("Calling LD client.waitForInitialization() ...");

    client.waitForInitialization();
  } catch (error) {
    console.log("ERROR: client.waitForInitialization failed");
  }

  console.log("Calling process.exit()");
  process.exit(0);
}

repro();

Expected behavior
The process would be terminated immediately.

Logs
The app prints the following, and then hangs for several seconds before exiting:

Initializing LD client ...
Calling LD client.waitForInitialization() ...
Calling process.exit()

SDK version
The latest version as of the current time:

  • launchdarkly-node-client-sdk@3.1.0
  • launchdarkly-js-sdk-common@5.1.0

Language version, developer tools
Node v20.2

OS/platform
MacOS 14.3

Additional context
I believe the problematic setTimeout calls are in these two locations:

periodicTimer = setTimeout(sendPeriodicEvent, periodicInterval);

processor.start = function() {
const flushTick = () => {
processor.flush();
flushTimer = setTimeout(flushTick, flushInterval);
};
flushTimer = setTimeout(flushTick, flushInterval);
};

@kinyoklion
Copy link
Member

@bvaughn Have you tried closing the SDK before exiting?

@bvaughn
Copy link
Author

bvaughn commented Apr 11, 2024

@kinyoklion Yes. I didn't include that in my repro because I was trying to trim it down as much as possible, but in our larger app– yes, we call close.

In this smaller project, it seems like calling close() is sufficient to fix the hang though, which is interesting.

@kinyoklion
Copy link
Member

Thank you @bvaughn,

Interesting. I would expect there could be a delay without closing the SDK. Closing should cancel most outstanding tasks. There could be tasks that are still running, like finishing up an in-progress http request, but I wouldn't expect it to consistently stall. I've filed a task to look into this.

Thank you,
Ryan

Filed internally as 240109

@kinyoklion
Copy link
Member

kinyoklion commented Apr 17, 2024

@bvaughn Are you only seeing this when the SDK was unable to initialize (no internet connection)? Or are you seeing it in longer running applications that have initialized as well?

Also have you only seen this on macOS?

@bvaughn
Copy link
Author

bvaughn commented Apr 18, 2024

@kinyoklion Only when there was no Internet to initialize in the first place. (Truthfully I haven’t tested the second scenario. Our use case is a CLI that is not typically a long-running process.)

@kinyoklion
Copy link
Member

Thank you @bvaughn

Unfortunately I have not been able to reproduce anything similar under any conditions so far. With or without networking or with or without closing the client. The above example always exits without delay.

Is this something you are seeing widely? Is this running on fleet machines where they all have uniform software? I am curious if a VPN or some other software could be needed in combination to exhibit the problem.

Apple Silicon or Intel mac?

Thank you,
Ryan

@bvaughn
Copy link
Author

bvaughn commented Apr 18, 2024

@kinyoklion No, it's not wide spread. We're only running this on a few machines so far, and I think we've seen it on two of them (both Apple M3 Pros).

I guess maybe the best thing for us to do for now is for us (me) to keep an eye on it to see if it's more widespread. I appreciate you taking a look though.

@kinyoklion
Copy link
Member

Thank you @bvaughn

I don't have an M3 to readily test, but I can at least try on an M1 (I was testing on an intel mac).

I have some suspicions around DNS resolution on macOS, but I don't know of a way to illicit the bad behavior. Theoretically it should fail nearly immediately in the no network case, but I have seen some oddities when multiple networks are available, for instance wifi is off, but there is a virtual network with a virtual machine or docker container.

Thanks,
Ryan

@kinyoklion
Copy link
Member

kinyoklion commented May 22, 2024

Hello @bvaughn,

I have released a new version of launchdarkly-eventsource which could impact this. https://github.com/launchdarkly/js-eventsource

A release of the node-client-sdk will be coming which incorporates this updated version. The issue was that in some situations a callback would get executed twice, which could schedule two reconnect timers and potentially additional network requests that were not closed when closing the client.

I am not 100% sure that this will impact your issue, but it at least can result in similar behavior, especially in cases with no network connection available.

Thank you,
Ryan

@bvaughn
Copy link
Author

bvaughn commented May 23, 2024

Thank you for the update, @kinyoklion!

@kinyoklion
Copy link
Member

Version 3.2.1 has been released which includes the event source changes.

Thank you,
Ryan

@kinyoklion
Copy link
Member

I am closing this for now. If the problem resurfaces, then it may be re-opened.

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