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

WIP still fixing up [v10.x backport] n-api: handle reference delete before finalize #25572

Closed
wants to merge 13 commits into from

Commits on Dec 26, 2018

  1. deps: cherry-pick 525b396 from V8 upstream

    Original commit message:
    
        [cpu-profiler] Fix a leak caused by re-logging existing functions.
    
        Don't re-log all existing functions during StartProcessorIfNotStarted().
        They will already be in the CodeMap attached to the ProfileGenerator and
        re-logging them causes leaks. See the linked bug for more details.
    
        Bug: v8:8253
        Change-Id: Ibb1a1ab2431c588e8c3a3a9ff714767cdf61a88e
        Reviewed-on: https://chromium-review.googlesource.com/1256763
        Commit-Queue: Peter Marshall <petermarshall@chromium.org>
        Reviewed-by: Yang Guo <yangguo@chromium.org>
        Cr-Commit-Position: refs/heads/master@{#56336}
    
    Refs: v8/v8@525b396
    
    PR-URL: nodejs#25041
    Reviewed-By: Yang Guo <yangguo@chromium.org>
    Reviewed-By: Rod Vagg <rod@vagg.org>
    psmarshall authored and MylesBorins committed Dec 26, 2018
    Configuration menu
    Copy the full SHA
    8810051 View commit details
    Browse the repository at this point in the history
  2. doc: update the http.request.setTimeout docs to be accurate

    Refs: nodejs#8895
    
    PR-URL: nodejs#25123
    Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
    Reviewed-By: James M Snell <jasnell@gmail.com>
    jbunton-atlassian authored and MylesBorins committed Dec 26, 2018
    Configuration menu
    Copy the full SHA
    3af7528 View commit details
    Browse the repository at this point in the history

Commits on Dec 30, 2018

  1. test: fix test-repl-envvars

    In 180f865, the test was changed
    so that the `env` argument of `createInternalRepl()` also contained
    external environment variables, because keeping them can be necessary
    for spawning processes on some systems.
    
    However, this test does not spawn new processes, and relies on the
    fact that the environment variables it tests are not already set
    (and fails otherwise); therefore, reverting to the original state
    should fix this.
    
    Fixes: nodejs#21451
    Fixes: nodejs/build#1377
    Refs: nodejs#25219
    
    PR-URL: nodejs#25226
    Reviewed-By: Rich Trott <rtrott@gmail.com>
    Reviewed-By: Tobias Nießen <tniessen@tnie.de>
    Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
    Reviewed-By: Denys Otrishko <shishugi@gmail.com>
    Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
    Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
    addaleax committed Dec 30, 2018
    Configuration menu
    Copy the full SHA
    48da8be View commit details
    Browse the repository at this point in the history

Commits on Jan 18, 2019

  1. n-api: add API for asynchronous functions

    Bundle a `uv_async_t`, a `uv_idle_t`, a `uv_mutex_t`, a `uv_cond_t`,
    and a `v8::Persistent<v8::Function>` to make it possible to call into JS
    from another thread. The API accepts a void data pointer and a callback
    which will be invoked on the loop thread and which will receive the
    `napi_value` representing the JavaScript function to call so as to
    perform the call into JS. The callback is run inside a
    `node::CallbackScope`.
    
    A `std::queue<void*>` is used to store calls from the secondary
    threads, and an idle loop is started by the `uv_async_t` callback on the
    loop thread to drain the queue, calling into JS with each item.
    
    Items can be added to the queue blockingly or non-blockingly.
    
    The thread-safe function can be referenced or unreferenced, with the
    same semantics as libuv handles.
    
    Re: nodejs/help#1035
    Re: nodejs#20964
    Fixes: nodejs#13512
    Backport-PR-URL: nodejs#25002
    PR-URL: nodejs#17887
    Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
    Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
    Gabriel Schulhof authored and MylesBorins committed Jan 18, 2019
    Configuration menu
    Copy the full SHA
    64f232c View commit details
    Browse the repository at this point in the history
  2. n-api: fix compiler warning

    private field 'async_context' is not used [-Wunused-private-field]
    
    PR-URL: nodejs#21597
    Refs: nodejs#17887
    Backport-PR-URL: nodejs#25002
    Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
    Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
    cjihrig authored and MylesBorins committed Jan 18, 2019
    Configuration menu
    Copy the full SHA
    ca8ba15 View commit details
    Browse the repository at this point in the history
  3. n-api: guard against cond null dereference

    A condition variable is only created by the thread-safe function if the
    queue size is set to something larger than zero. This adds null-checks
    around the condition variable and tests for the case where the queue
    size is zero.
    
    Fixes: nodejs/help#1387
    PR-URL: nodejs#21871
    Backport-PR-URL: nodejs#25002
    Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
    Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
    Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
    Gabriel Schulhof authored and MylesBorins committed Jan 18, 2019
    Configuration menu
    Copy the full SHA
    7b68d89 View commit details
    Browse the repository at this point in the history
  4. src: fix may be uninitialized warning in n-api

    Backport-PR-URL: nodejs#25002
    PR-URL: nodejs#21898
    Reviewed-By: Anna Henningsen <anna@addaleax.net>
    Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
    Reviewed-By: James M Snell <jasnell@gmail.com>
    Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
    Reviewed-By: Jon Moss <me@jonathanmoss.me>
    Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
    mhdawson authored and MylesBorins committed Jan 18, 2019
    Configuration menu
    Copy the full SHA
    c275534 View commit details
    Browse the repository at this point in the history
  5. n-api: remove idle_running from TsFn

    The idle_running member variable in TsFn is always false and can
    therefore be removed.
    
    Backport-PR-URL: nodejs#25002
    PR-URL: nodejs#22520
    Reviewed-By: James M Snell <jasnell@gmail.com>
    Reviewed-By: Anna Henningsen <anna@addaleax.net>
    Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
    Reviewed-By: Gus Caplan <me@gus.host>
    ralphtheninja authored and MylesBorins committed Jan 18, 2019
    Configuration menu
    Copy the full SHA
    311e1f1 View commit details
    Browse the repository at this point in the history
  6. n-api: clean up thread-safe function

    * Move class `TsFn` to name space `v8impl` and rename it to
      `ThreadSafeFunction`
    * Remove `NAPI_EXTERN` from API declarations, because it's only needed
      in the header file.
    
    Backport-PR-URL: nodejs#25002
    PR-URL: nodejs#22259
    Reviewed-By: Anna Henningsen <anna@addaleax.net>
    Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
    Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
    Gabriel Schulhof authored and MylesBorins committed Jan 18, 2019
    Configuration menu
    Copy the full SHA
    a0fa101 View commit details
    Browse the repository at this point in the history
  7. doc: fix optional parameters in n-api.md

    The thread_finalize_data and thread_finalize_cb parameters in
    napi_create_threadsafe_function are optional.
    
    Backport-PR-URL: nodejs#25002
    PR-URL: nodejs#22998
    Reviewed-By: James M Snell <jasnell@gmail.com>
    Reviewed-By: Anna Henningsen <anna@addaleax.net>
    ralphtheninja authored and MylesBorins committed Jan 18, 2019
    Configuration menu
    Copy the full SHA
    ffbfe34 View commit details
    Browse the repository at this point in the history
  8. n-api: add missing handle scopes

    Currently when building with --debug
    test/addons-napi/test_threadsafe_function will error:
    
    $  out/Debug/node test/addons-napi/test_threadsafe_function/test.js
    FATAL ERROR: v8::HandleScope::CreateHandle()
      Cannot create a handle without a HandleScope
     1: 0x10004e287 node::DumpBacktrace(__sFILE*) [node/out/Debug/node]
     2: 0x1000cd37b node::Abort() [/node/out/Debug/node]
     3: 0x1000cd69f node::OnFatalError(char const*, char const*)
        [/node/out/Debug/node]
     4: 0x1004df0b1 v8::Utils::ReportApiFailure(char const*, char const*)
        [/nodejs/node/out/Debug/node]
     5: 0x100a8c0a9 v8::internal::HandleScope::Extend(
            v8::internal::Isolate*)
        [/node/out/Debug/node]
     6: 0x1004e4229 v8::EmbedderDataFor(v8::Context*,
                                        int, bool,
                                        char const*)
        [/node/out/Debug/node]
     7: 0x1004e43fa v8::Context::SlowGetAlignedPointerFromEmbedderData(int)
        [/node/out/Debug/node]
     8: 0x10001c26b v8::Context::GetAlignedPointerFromEmbedderData(int)
        [/node/out/Debug/node]
     9: 0x1000144ea node::Environment::GetCurrent(v8::Local<v8::Context>)
        [/node/out/Debug/node]
    10: 0x1000f49e2 napi_env__::node_env() const
        [/node/out/Debug/node]
    11: 0x1000f9885
        (anonymous namespace)::v8impl::ThreadSafeFunction::
            CloseHandlesAndMaybeDelete(bool)
        [/node/out/Debug/node]
    12: 0x1000fb34f (anonymous namespace)::v8impl::ThreadSafeFunction::
            DispatchOne()
        [/node/out/Debug/node]
    13: 0x1000fb129
        (anonymous namespace)::v8impl::ThreadSafeFunction::
            IdleCb(uv_idle_s*)
        [/node/out/Debug/node]
    14: 0x1011a1b69 uv__run_idle
        [/node/out/Debug/node]
    15: 0x101198179 uv_run
        [/node/out/Debug/node]
    16: 0x1000dfca1
        node::Start(...)
        [/node/out/Debug/node]
    17: 0x1000dae50 node::Start(...)
        [/node/out/Debug/node]
    18: 0x1000da56f node::Start(int, char**)
        [/node/out/Debug/node]
    19: 0x10141112e main
        [/node/out/Debug/node]
    20: 0x100001034 start
        [/node/out/Debug/node]
    Abort trap: 6
    
    This commit adds two HandleScope's, one to CloseHandlesAndMaybeDelete
    and one to the lambda.
    
    SlowGetAlignedPointerFromEmbedderData will only be called for debug
    builds:
    https://github.com/v8/v8/blob/2ef0aa662fe907a1b36ac1abe7d77ad2bcd27733
    /include/v8.h#L10440-L10447
    
    Backport-PR-URL: nodejs#25002
    PR-URL: nodejs#24011
    Reviewed-By: James M Snell <jasnell@gmail.com>
    Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
    danbev authored and MylesBorins committed Jan 18, 2019
    Configuration menu
    Copy the full SHA
    cffe85c View commit details
    Browse the repository at this point in the history
  9. test: mark test_threadsafe_function/test as flaky

    The test fails consistently on windows-fanned with vs2017.
    mark it as flaky while the issue is being progressed, and
    to keep CI green / amber.
    
    Ref: nodejs#23621
    Backport-PR-URL: nodejs#25002
    PR-URL: nodejs#24714
    Reviewed-By: James M Snell <jasnell@gmail.com>
    Reviewed-By: Rich Trott <rtrott@gmail.com>
    gireeshpunathil authored and MylesBorins committed Jan 18, 2019
    Configuration menu
    Copy the full SHA
    b44bd88 View commit details
    Browse the repository at this point in the history
  10. n-api: handle reference delete before finalize

    Crashes were reported during finalization due to
    the memory for a reference being deleted and the
    finalizer running after the deletion.
    
    This change ensures the deletion of the memory for
    the reference only occurs after the finalizer has run.
    
    Fixes: nodejs/node-addon-api#393
    
    Backport-PR-URL: nodejs#25572
    PR-URL: nodejs#24494
    Reviewed-By: Anna Henningsen <anna@addaleax.net>
    Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
    Reviewed-By: Refael Ackermann <refack@gmail.com>
    mhdawson committed Jan 18, 2019
    Configuration menu
    Copy the full SHA
    4f1aeb3 View commit details
    Browse the repository at this point in the history