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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unref'd timeouts should not be reported as open handles #8939

Closed
pimterry opened this issue Sep 11, 2019 · 5 comments 路 Fixed by #8941
Closed

Unref'd timeouts should not be reported as open handles #8939

pimterry opened this issue Sep 11, 2019 · 5 comments 路 Fixed by #8941

Comments

@pimterry
Copy link
Contributor

馃悰 Bug Report

Timeout.unref() in node tells node not to let the timeout block process shutdown. That means that these timeouts are never going to stop Jest from shutting down, but --detectOpenHandles still reports them as such.

Any intervals or timeouts that has been unref'd should not be reported by Jest as an open handle.

I'm not actually a Jest user myself - I'm reporting this as I received a bug report on a testing library I maintain, because Jest is complaining about an unref'd interval used here.

To Reproduce

In any test, include an unref'd timer:

const interval = setInterval(() => {}, 10000);
interval.unref();

This test will exit happily with no problems.

Run this test without the unref() line though and it won't, and you'll see a Jest did not exit one second after the test run has completed warning.

However, if you run this test with --detectOpenHandles then regardless of unref(), this setInterval will always be reported as an open handle.

Expected behavior

--detectOpenHandles should not show a warning for this interval, but it does.

Suggestion

Since Node v11, there's a Timeout.hasRef() function, which can be used to check whether any timeout will keep the event loop active, or whether it's unref'd and will allow shutdown.

I'd be happy to put together a quick PR to fix this, if it's likely to be accepted? I think this is pretty easy to solve for node 11+: just filter the active timeout handles by the result of hasRef(), if present.

You could polyfill hasRef() for pre-v11 versions, but v12 is going to be the active LTS release in a month anyway, and this isn't a blocking issue, so I probably wouldn't bother.

How does that sound?

@jeysal
Copy link
Contributor

jeysal commented Sep 11, 2019

How does that sound?

Awesome! Agree that this is a bug and we should check hasRef, and also that if there is no hasRef, we should just keep the current behavior and wait for the old Node versions to be discontinued.

@jeysal
Copy link
Contributor

jeysal commented Sep 11, 2019

In case it saves you time, collectHandles.ts is probably where you need to look :)

@pimterry
Copy link
Contributor Author

@jeysal great, thanks! PR here: #8941

@icsaba
Copy link

icsaba commented Oct 8, 2020

I am using jest 26.4.2 and the same issue just appeared when i use jest.useFakeTimers(). do you have any suggestions?

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants