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

Environment class never cleans up UV handles except in cctest? #20517

Closed
MSLaguana opened this issue May 4, 2018 · 2 comments
Closed

Environment class never cleans up UV handles except in cctest? #20517

MSLaguana opened this issue May 4, 2018 · 2 comments
Labels
libuv Issues and PRs related to the libuv dependency or the uv binding.

Comments

@MSLaguana
Copy link
Contributor

  • Version: 10.0.0
  • Platform: windows
  • Subsystem: env

While trying to improve the reliability of cctest in node-chakracore, I noticed that the node::Environment class sets up some libuv handles (e.g. the idle_check_handle_ and immediate_check_handle_) and registers them to be properly closed and cleaned up whenever Environment::FreeEnvironment is called, but that function is never run in a normal node session. The only time that function is invoked is during cctest.

Is this intentional? I ran into this because I was trying to make sure that when a node-chakracore Isolate is destructed it cleans up its own libuv handles, but when I was doing so I keep getting segfaults caused by libuv trying to use some of the Environment handles that had been freed but not cleaned up by libuv.
@MSLaguana MSLaguana mentioned this issue May 4, 2018
Closed
4 tasks
@addaleax addaleax added the libuv Issues and PRs related to the libuv dependency or the uv binding. label May 4, 2018
@addaleax
Copy link
Member

addaleax commented May 4, 2018

I think #19377 is basically the fix for this issue? I’ll add a Fixes: tag there if that seems okay.

@MSLaguana
Copy link
Contributor Author

That does look like it addresses this issue. Thanks!

addaleax added a commit to addaleax/node that referenced this issue May 9, 2018
@addaleax
src: keep track of open requests
7e4114d 
Workers cannot shut down while requests are open, so keep a counter
that is increased whenever libuv requests are made and decreased
whenever their callback is called.

This also applies to other embedders, who may want to shut down
an `Environment` instance early.

Many thanks for Stephen Belanger for reviewing the original version of
this commit in the Ayo.js project.

Fixes: nodejs#20517
Refs: ayojs/ayo#85
addaleax added a commit that referenced this issue May 14, 2018
@addaleax
src: keep track of open requests
8995408 
Workers cannot shut down while requests are open, so keep a counter
that is increased whenever libuv requests are made and decreased
whenever their callback is called.

This also applies to other embedders, who may want to shut down
an `Environment` instance early.

Many thanks for Stephen Belanger for reviewing the original version of
this commit in the Ayo.js project.

Fixes: #20517
Refs: ayojs/ayo#85
PR-URL: #19377
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
libuv Issues and PRs related to the libuv dependency or the uv binding.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@addaleax @MSLaguana