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

Connections remain open after shut down #1080

Closed
JrSchild opened this issue Oct 22, 2019 · 7 comments · Fixed by #1083
Closed

Connections remain open after shut down #1080

JrSchild opened this issue Oct 22, 2019 · 7 comments · Fixed by #1083

Comments

@JrSchild
Copy link
Contributor

JrSchild commented Oct 22, 2019

Problem description

Connections remain open after invoking tryShutdown and forceShutdown in tests.

Reproduction steps

git clone [email protected]:JrSchild/grpc-open-connetions.git grpc-open-connetions
cd grpc-open-connetions
npm install
npm run test

Which produces the following output:

 PASS  ./test.spec.js
  ✓ should shut the server down (2ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        0.376s, estimated 1s
Ran all test suites matching /test.spec.js/i.

Jest has detected the following 1 open handle potentially keeping Jest from exiting:

  ●  Timeout

      at new SubchannelPool (node_modules/@grpc/grpc-js/build/src/subchannel-pool.js:38:13)
      at Object.<anonymous> (node_modules/@grpc/grpc-js/build/src/subchannel-pool.js:99:30)

Our tests require reconnecting with grpc. Unfortunately we cannot reliably test this because the connection remains open after shutting down.
Edit: The setInterval created in subchannel-pool is never cleaned up. It could maybe be fixed by clearing the interval when the pool is empty and starting it again when there is 0 < channelTargets.

Environment

  • OS name, version and architecture: [Mac OS X 10.14.5]
  • Node version [12.6.0]
  • Node installation method [nvm]
  • Package name and version [[email protected]]
@murgatroid99
Copy link
Member

That setInterval is explicitly unrefed so that the process can exit even while that timer is active. Jest may not be properly detecting that. All connections are also unrefed as long as they do not have any active requests.

@JrSchild
Copy link
Contributor Author

Thank you @murgatroid99, I will verify this tomorrow.

@merlinnot
Copy link
Contributor

@murgatroid99 I believe there should be an API that allows users to close the created server. Unref-ing only works if users intend to exit the entire process, but that might not always be the case.

@murgatroid99
Copy link
Member

First, this is about client connections. Servers are handled differently. Second, connections are automatically closed if no open clients are using them, and clients can be explicitly closed. And third, this issue is about things holding the process open. If the user doesn't want to close the process, there's no issue.

@merlinnot
Copy link
Contributor

s/server/client/

Sorry, it was late in my time zone 🙃

I have a similar issue with Firestore and “connections are automatically closed if no open clients are using them” is in contradiction with googleapis/nodejs-firestore#769 (comment). Could you elaborate on it a little more?

@merlinnot
Copy link
Contributor

Does “using” mean that you have to close the client, or just don’t perform any operations?

@murgatroid99
Copy link
Member

I mean that connections are closed if no clients hold references to them. But since connections are also unrefed as long as there are no active requests, they still shouldn't hold the process open.

I am also describing the functionality of the latest version of grpc-js, which I believe had not been picked up by Firestore at the time that you filed the linked issue.

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

Successfully merging a pull request may close this issue.

3 participants