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.

Sign up for GitHub

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
Jump to bottom

Timer objects no longer show up in process._getActiveHandles() #25806

Closed
isaacs opened this issue Jan 30, 2019 · 7 comments
Closed

Timer objects no longer show up in process._getActiveHandles() #25806

isaacs opened this issue Jan 30, 2019 · 7 comments
Labels
timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. wontfix Issues that will not be fixed.

Comments

@isaacs
Copy link
Contributor

Up until node v10, process._getActiveHandles() would show Timer objects that were keeping the process open.

For example:

$ node -v
v10.15.1

$ node -p 'setTimeout(_=>_,500);process._getActiveHandles().filter(h=>h!=process.stdout && h !=process.stderr)'
[ Timer {
    _list:
     TimersList {
       _idleNext: [Timeout],
       _idlePrev: [Timeout],
       _unrefed: false,
       msecs: 500,
       _timer: [Circular] } } ]

However, in Node v11, this is not the case:

$ node -v
v11.8.0

$ node -p 'setTimeout(_=>_,500);process._getActiveHandles().filter(h=>h!=process.stdout && h !=process.stderr)'
[]

Is there a better or more canonical way to get insight into what's keeping the process from exiting gracefully? Or is this a bug?
isaacs added a commit to tapjs/tapjs that referenced this issue Jan 30, 2019
@isaacs
Ignore timers in _getActiveHandles reporting
88eb85b 
Re: nodejs/node#25806
@richardlau
Copy link
Member

I think this is due to #20894 (#20894 (review)).

cc @nodejs/timers @nodejs/diagnostics

@Fishrock123
Copy link
Member

Fishrock123 commented Jan 30, 2019 •
edited

Yes, it is due to the aforementioned and related commits.

The tl;dr has apparently been that no one really cares about _getActiveHandles(). This also broke some stuff internally at NodeSource at the time.

EDIT: to be clear, this is because all of the timer's work aside from the single base timer handle is now all done in javascript, and also detached form the handle for efficiency with V8.

See #21453 for a potential public version of this api, although really I think timers should maybe be a separate api, and continuations (nextTick, MTQ, setImmediate) an additional api still.

@Fishrock123 Fishrock123 added the timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. label Jan 30, 2019
@isaacs
Copy link
Contributor Author

@Fishrock123 Thanks for the link.

The tl;dr has apparently been that no one really cares about _getActiveHandles().

I care :(

But yeah, it's always been a jank API. I'd be thrilled to see something better in that space.

What's the likelihood of getting Timers brought back in some fashion in _getActiveHandles()? This seems like a bug to me.

@Fishrock123
Copy link
Member

Fishrock123 commented Jan 31, 2019 •
edited

The tl;dr has apparently been that no one really cares about _getActiveHandles().

I care :(

I mean I'd like to say that I do too but it's been like 9 months. And, well, I want to get time to look at this again from a public api soon but, I say that a lot.

I think it is unlikely that anyone will want to change/fix/whatever _getActiveHandles(). I don't 100% remember the context around the discussions at the time, though.

@vmarchaud
Copy link
Contributor

vmarchaud commented Feb 2, 2019 •
edited

I believe those APIs are mainly used by APMs vendor, we got the issue too at @keymetrics (keymetrics/pm2-io-apm#213) and found a somewhat fix

@mmarchini
Copy link
Contributor

To add to this discussion, llnode has it's own getactivehandles/getactiverequest commands, and while upgrading llnode to work with Node.js v11 some tests stopped working because of this change.

@mmarchini mmarchini mentioned this issue Feb 22, 2019
Closed
3 tasks
@mariodu mariodu mentioned this issue Feb 20, 2020
Merged
@apapirovski apapirovski added the wontfix Issues that will not be fixed. label Feb 24, 2023
@apapirovski
Copy link
Member

Given it has been 3+ years, at this point it seems like we should close and move on. Re-introducing is unlikely to add any value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. wontfix Issues that will not be fixed.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@isaacs @apapirovski @Fishrock123 @vmarchaud @mmarchini @richardlau