async_hooks: tear down hooks if none are in use

[addaleax]

Basically, reset everything into its original state once all
async hooks have been disabled as closely as possible.

This has been requested by Andreas, and I agree that it makes sense.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included (how do you test that nothing happens?)
• commit message follows commit guidelines

Affected core subsystem(s)

async_hooks

/cc @nodejs/async_hooks

[AndreasMadsen]

Thanks.

LGTM, but I would like another review.

[AndreasMadsen]

I think we have to solve #13427 before we can think about tear down.

edit: it would be lovely if one could delete review comments :/

I think we have to solve #13427 before we can think about tear down.

[matthewloring]

The approach seems good. Was there a particular use case you had in mind for dynamically disabling these hooks?

[addaleax]

Was there a particular use case you had in mind for dynamically disabling these hooks?

No, but that applies to .disable() in general; I don’t really know why people would want to, but I’m sure there’s a reason for that.

[matthewloring]

Yup, agreed. I was just curious if there was a specific need.

[trevnorris]

There was no reason for 410b141 and there isn't any reason for this. There is zero extra overhead for setting the functions early, but there is overhead for setting them again.

[trevnorris]

I'll have a PR ready soon to fix this.

[trevnorris]

@addaleax please excuse the rudeness of my earlier comment. i've been preoccupied with other work and wasn't involved as much the last couple weeks as I should have been. the comment was a reaction at my frustration with myself for this reason. it wasn't meant for you. i'm sorry.

[addaleax]

@trevnorris Don’t worry about it, I didn’t even catch up to read it until now. 😄 Your PR seems just as fine to me, I think this can be closed.