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

(v7.x backport) src: make AtExit callback's per Environment #12664

Closed
wants to merge 4 commits into from
Closed

(v7.x backport) src: make AtExit callback's per Environment #12664

wants to merge 4 commits into from

Commits on May 2, 2017

  1. build: enable cctest to use generated objects 

    This commit tries to make it simpler to add unit tests (cctest) for
code that needs to test node core funtionality but that might not be
appropriate as an addon or a JavaScript test. An example of this could
be adding functionality targeted for situations when Node itself is
embedded.

Currently it was not as easy, or efficient, as one would have hoped to
add such tests. The object output directories vary for different
operating systems which we need to link to so that we don't have an
additional compilation step.

PR-URL: nodejs#11956
Ref: nodejs#9163
Reviewed-By: James M Snell <jasnell@gmail.com>
    @danbev
    danbev committed May 2, 2017
    Configuration menu
    Copy the full SHA
    d7ea036 View commit details
    Browse the repository at this point in the history

  2. src: don't call uv_run() after 'exit' event 

    It makes timers and other libuv handles fire intermittently after the
'exit' event, contrary to what the documentation states.

Regression introduced in commit aac79df ("src: use stack-allocated
Environment instances") from June last year that made the
`while (handle_cleanup_waiting_ != 0) uv_run(event_loop(), UV_RUN_ONCE)`
loop run unconditionally on exit because it merged CleanupHandles() into
the Environment destructor.

This change breaks parallel/test-async-wrap-throw-from-callback because
the async_wrap idle handle is no longer cleaned up, which I resolved
pragmatically by removing the test.

In all seriousness, it is being removed in the upcoming async_wrap
revamp - it doesn't make sense to sink a lot of time in it now.

Fixes: nodejs#12322
PR-URL: nodejs#12344
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
    @bnoordhuis @danbev
    bnoordhuis authored and danbev committed May 2, 2017
    Configuration menu
    Copy the full SHA
    3fb8537 View commit details
    Browse the repository at this point in the history

  3. src: make AtExit callback's per Environment 

    This commit attempts to address one of the TODOs in
nodejs#4641 regarding making the
AtExit callback's per environment, instead of the current global.

bnoordhuis provided a few options for solving this, and one was to
use a thread-local which is what this commit attempts to do.

PR-URL: nodejs#9163
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
    @danbev
    danbev committed May 2, 2017
    Configuration menu
    Copy the full SHA
    565a6a3 View commit details
    Browse the repository at this point in the history

  4. test: cleanup handles in test_environment 

    The test fixtures create multiple node::Environments that all use the
uv_default_loop(), and since the test does not clean up the handles
created by Environment::Start(), the default libuv loop structure
contains dangling pointers after the first Environment is freed,
which then means that creating new handles leads to memory corruption.

PR-URL: nodejs#12621
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
    @addaleax @danbev
    addaleax authored and danbev committed May 2, 2017
    Configuration menu
    Copy the full SHA
    ed545be View commit details
    Browse the repository at this point in the history