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

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

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

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

isaacs opened this issue Jan 30, 2019 · 8 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

isaacs commented Jan 30, 2019

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
@richardlau
Copy link
Member

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

cc @nodejs/timers @nodejs/diagnostics

@Fishrock123
Copy link
Contributor

Fishrock123 commented Jan 30, 2019

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

isaacs commented Jan 30, 2019

@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
Contributor

Fishrock123 commented Jan 31, 2019

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

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.

@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.

@kingshukbasak
Copy link

Hi, Having this feature can add a lot of value when there is a memory leak in an enterprise system due to increase in Active Handlers/Request. Hope the method is back.

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

No branches or pull requests

7 participants