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

[v8.x backport] Backport tsfn to v8.x #25002

Commits on Dec 28, 2018

  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
    PR-URL: nodejs#17887
    Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
    Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
    Gabriel Schulhof committed Dec 28, 2018
    Configuration menu
    Copy the full SHA
    2e15372 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
    Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
    Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
    cjihrig authored and Gabriel Schulhof committed Dec 28, 2018
    Configuration menu
    Copy the full SHA
    b285f1d 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
    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 committed Dec 28, 2018
    Configuration menu
    Copy the full SHA
    7600c04 View commit details
    Browse the repository at this point in the history
  4. src: fix may be uninitialized warning in n-api

    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 Gabriel Schulhof committed Dec 28, 2018
    Configuration menu
    Copy the full SHA
    7965765 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.
    
    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 Gabriel Schulhof committed Dec 28, 2018
    Configuration menu
    Copy the full SHA
    46118be 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.
    
    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 committed Dec 28, 2018
    Configuration menu
    Copy the full SHA
    35c6b1d 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.
    
    PR-URL: nodejs#22998
    Reviewed-By: James M Snell <jasnell@gmail.com>
    Reviewed-By: Anna Henningsen <anna@addaleax.net>
    ralphtheninja authored and Gabriel Schulhof committed Dec 28, 2018
    Configuration menu
    Copy the full SHA
    8efb65b 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
    
    PR-URL: nodejs#24011
    Reviewed-By: James M Snell <jasnell@gmail.com>
    Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
    danbev authored and Gabriel Schulhof committed Dec 28, 2018
    Configuration menu
    Copy the full SHA
    f116dc4 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
    PR-URL: nodejs#24714
    Reviewed-By: James M Snell <jasnell@gmail.com>
    Reviewed-By: Rich Trott <rtrott@gmail.com>
    gireeshpunathil authored and Gabriel Schulhof committed Dec 28, 2018
    Configuration menu
    Copy the full SHA
    e0e1ac2 View commit details
    Browse the repository at this point in the history

Commits on Jan 2, 2019

  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 authored and Gabriel Schulhof committed Jan 2, 2019
    Configuration menu
    Copy the full SHA
    f2e4510 View commit details
    Browse the repository at this point in the history