Commit Graph
20 Commits
Author SHA1 Message Date
Jason Ginchereau 0c258bdc40 n-api: Context for custom async operations
- Add napi_async_context opaque pointer type.
   (If needed, we could later add APIs for getting the async IDs
   out of this context.)
 - Add napi_async_init() and napi_async_destroy() APIs.
 - Add async_context parameter to napi_make_callback().
 - Add code and checks to test_make_callback to validate async context
   APIs by checking async hooks are called with correct context.
 - Update API documentation.

PR-URL: https://github.com/nodejs/node/pull/15189
Fixes: https://github.com/nodejs/node/issues/13254
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2017-09-14 16:05:46 -04:00
Jason Ginchereau 1a0727d85e n-api: change async resource name to napi_value
PR-URL: https://github.com/nodejs/node/pull/14697
Reviewed-By: Anna Henningsen <anna@addaleax.net>
2017-09-14 17:39:05 +02:00
Jason Ginchereau af70c3b478 n-api: optimize number API performance
- Add separate APIs for creating different kinds of numbers,
   because creating a V8 number value from an integer is faster
   than creating one from a double.
 - When getting number values, avoid getting the current context
   because the context will not actually be used and is expensive
   to obtain.
 - When creating values, don't use v8::TryCatch (NAPI_PREAMBLE),
   because these functions have no possibility of executing JS code.

Refs: https://github.com/nodejs/node/issues/14379
PR-URL: https://github.com/nodejs/node/pull/14573
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
2017-08-07 17:31:17 -07:00
Jason Ginchereau cee8d6d65e build,win: fix python detection script
Handle spaces in the path to python.exe, in case it is installed
under some directory like "C:\Program Files".

PR-URL: https://github.com/nodejs/node/pull/14546
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
2017-08-01 10:56:04 -07:00
Jason Ginchereau ddba969321 test: fix build warning in addons-napi/test_object
PR-URL: https://github.com/nodejs/node/pull/13412
Reviewed-By: Anna Henningsen <anna@addaleax.net>
2017-06-05 16:34:13 -07:00
Jason Ginchereau 8ab8c33985 n-api: enable napi_wrap() to work with any object
Previously, napi_wrap() would only work with objects created from a
constructor returned by napi_define_class(). While the N-API team
was aware of this limitation, it was not clearly documented and is
likely to cause confusion anyway. It's much simpler if addons are
allowed to use any JS object. Also, the specific behavior of the
limitation is difficult to reimplement on other VMs that work
differently from V8.

V8 requires object internal fields to be declared on the object
prototype (which napi_define_class() used to do). Since it's too
late to modify the object prototype by the time napi_wrap() is
called, napi_wrap() now inserts a new object (with the internal
field) into the supplied object's prototype chain. Then it can be
retrieved from there later by napi_unwrap().

This change also includes improvements to the documentation for
napi_create_external(), partly to explain how it is different from
napi_wrap().

PR-URL: https://github.com/nodejs/node/pull/13250
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
2017-06-01 11:46:55 -07:00
Jason Ginchereau bb91879f31 test: Make N-API weak-ref GC tests asynchronous
One of the N-API weak-reference test cases already had to be made
asynchronous to handle different behavior in a newer V8 version:
https://github.com/nodejs/node/pull/12864
When porting N-API to Node-ChakraCore, we found more of the test
cases needed similar treatment:
https://github.com/nodejs/node-chakracore/issues/246 So to make
thes tests more robust (and avoid having differences in the test code
for Node-ChakraCore), I am refactoring the tests in this file to
insert a `setImmedate()` callback before every call to `gc()` and
assertions about the effects of the GC.

PR-URL: https://github.com/nodejs/node/pull/13121
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
2017-05-30 12:45:27 -07:00
Jason Ginchereau f5df94bf78 doc: add jasongin & kunalspathak to collaborators
PR-URL: https://github.com/nodejs/node/pull/13200
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
2017-05-24 10:58:50 -07:00
Jason Ginchereau a63b245b0a n-api: Retain last code when getting error info
Unlike most N-API functions, `napi_get_last_error_info()` should not
clear the last error code when successful, because a pointer to (not
a copy of) the error info structure is returned via an out parameter.

PR-URL: https://github.com/nodejs/node/pull/13087
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
2017-05-19 17:04:02 -04:00
Jason Ginchereau 2e3fef7628 n-api: Handle fatal exception in async callback
- Create a handle scope before invoking the async completion
   callback, because it is basically always needed, easy for user
   code to forget, and this makes it more consistent with ordinary
   N-API function callbacks.

 - Check for an unhandled JS exception after invoking an async
   completion callback, and report it via `node::FatalException()`.

 - Add a corresponding test case for an exception in async callback.

Previously, any unhandled JS exception thrown from a
`napi_async_complete_callback` would be silently ignored. Among other
things this meant assertions in some test cases could be undetected.

PR-URL: https://github.com/nodejs/node/pull/12838
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2017-05-09 15:17:36 -04:00
Jason Ginchereau 2bbabb1f85 n-api: napi_get_cb_info should fill array
When the number of args requested is greater than the actual number of
args supplied to the function call, the remainder of the args array
should be filled in with `undefined` values. Because of this bug, the
remainder of the array was left uninitialized, which could cause a
crash.

Refer to the documentation for the `argv` parameter at
https://github.com/nodejs/node/blob/master/doc/api/n-api.md#napi_get_cb_info

PR-URL: https://github.com/nodejs/node/pull/12863
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
2017-05-09 15:12:15 -04:00
Jason Ginchereau 972bfe1514 n-api: Sync with back-compat changes
Background: To enable N-API support for node versions back to v4, the
N-API code can also be built as an external addon. To make maintenance
easier, a single codebase needs to support both built-in and external
scenarios, along with Node versions >= 4 (and corresponding V8
versions).

This change includes several minor fixes to avoid using node
internal APIs and support older V8 versions:
  - Expand node::arraysize
  - In the CHECK_ENV() macro, return an error code instead of calling
    node::FatalError(). This is more consistent with how other invalid
    arguments to N-API functions are handled.
  - In v8impl::SetterCallbackWrapper::SetReturnValue(), do nothing
    instead of calling node::FatalError(). This is more consistent
    with JavaScript setter callbacks, where any returned value is
    silently ignored.
  - When queueing async work items, get the uv default loop instead of
    getting the loop from node::Environment::GetCurrent(). Currently
    that returns the same loop anyway. If/when node supports multiple
    environments, it should have a public API for getting the
    environment & event loop, and we can update this implementation
    then.
  - Use v8::Maybe::FromJust() instead of the newer alias ToChecked()

PR-URL: https://github.com/nodejs/node/pull/12674
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
2017-04-28 09:33:34 -04:00
Jason Ginchereau 427125491f n-api: Reference and external tests
- Add a test project to addons-napi that covers the
   N-API reference and external APIs
 - Fix a bug in napi_typeof that was found by the new tests

PR-URL: https://github.com/nodejs/node/pull/12551
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
2017-04-27 16:33:19 -04:00
Jason Ginchereau b7a341d7e5 n-api: Enable scope and ref APIs during exception
N-API is somewhat strict about blocking calls to many APIs while there
is a pending exception. The NAPI_PREAMBLE macro at the beginning of
many API implementations checks for a pending exception. However, a
subset of the APIs (which don't call back into JavaScript) still need
to work while in a pending-exception state. This changes the reference
APIs (equivalent to v8::Persistent) and handle scope APIs so that they
can be used for cleanup up while an exception is pending.

We may decide to similarly enable a few other APIs later, (which would
be a non-breaking change) but we know at least these are needed now
to unblock some specific scenarios.

Fixes: https://github.com/nodejs/abi-stable-node/issues/122
Fixes: https://github.com/nodejs/abi-stable-node/issues/228
PR-URL: https://github.com/nodejs/node/pull/12524
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
2017-04-25 22:02:56 +02:00
Jason Ginchereau 468275ac79 n-api: remove napi_get_value_string_length()
This API doesn't serve much purpose, and is only likely to cause
confusion and bugs. The intention was that this would return the
number of characters in a string independent of encoding, but
that's not generally useful. In almost all cases, one of the
encoding-specific napi_get_value_string_* APIs is more correct.
(Pass a null buffer if only the encoded length is desired.)

Anyway the current implementation of napi_get_value_string_length()
is technically wrong: it returns the number of 2-byte code units of
the UTF-16 encoding, but there are actually some characters that
are encoded as two UTF-16 code units.

Note the JavaScript String.prototype.length property returns the
number of UTF-16 code units, which may be different from the number
of characters. So, getting the true character count is not common
with JavaScript, and is probably best left to specialized
internationalization libraries.

PR-URL: https://github.com/nodejs/node/pull/12496
Fixes: https://github.com/nodejs/abi-stable-node/issues/226
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2017-04-20 13:27:17 -04:00
Jason Ginchereau 84602845c6 n-api: Update property attrs enum to match JS spec
The napi_property_attributes enum used names and values from
v8::PropertyAttribute, but those negative flag names were outdated
along with the default behavior of a property being writable,
enumerable, and configurable unless otherwise specified. To match the
ES5 standard property descriptor those attributes should be positive
flags and should default to false unless otherwise specified.

PR-URL: https://github.com/nodejs/node/pull/12240
Fixes: https://github.com/nodejs/abi-stable-node/issues/221
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
2017-04-07 09:40:21 -04:00
Jason Ginchereau 56e881d0b0 n-api: add support for abi stable module API
Add support for abi stable module API (N-API) as "Experimental feature".
The goal of this API is to provide a stable Node API for native
module developers. N-API aims to provide ABI compatibility guarantees
across different Node versions and also across different
Node VMs - allowing N-API enabled native modules to just work
across different versions and flavors of Node.js without recompilation.

A more detailed introduction is provided in:
https://github.com/nodejs/node-eps/blob/master/005-ABI-Stable-Module-API.md
and https://github.com/nodejs/abi-stable-node/blob/doc/VM%20Summit.pdf.

The feature, during its experimental state, will be guarded by a runtime
flag "--napi-modules". Only when this flag is added to the command line
will N-API modules along with regular non N-API modules be supported.

The API is defined by the methods in "src/node_api.h" and
"src/node_api_types.h". This is the best
starting point to review the API surface. More documentation will follow.

In addition to the implementation of the API using V8, which is included
in this PR, the API has also been validated against chakracore and that
port is available in
https://github.com/nodejs/abi-stable-node/tree/api-prototype-chakracore-8.x.

The current plan is to provide N-API support in versions 8.X and 6.X
directly. For older versions, such as 4.X or pre N-API versions of 6.X,
we plan to create an external npm module to provide a migration path
that will allow modules targeting older Node.js versions to use the API,
albeit without getting the advantage of not having to recompile.

In addition, we also plan an external npm package with C++ sugar to
simplify the use of the API. The sugar will be in-line only and will
only use the exported N-API methods but is not part of the N-API
itself. The current version is in:
https://github.com/nodejs/node-api.

This PR is a result of work in the abi-stable-node repo:
https://github.com/nodejs/abi-stable-node/tree/doc,
with this PR being the cumulative work on the api-prototype-8.x
branch with the following contributors in alphabetical order:

Author: Arunesh Chandra <arunesh.chandra@microsoft.com>
Author: Gabriel Schulhof <gabriel.schulhof@intel.com>
Author: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Author: Ian Halliday <ianhall@microsoft.com>
Author: Jason Ginchereau <jasongin@microsoft.com>
Author: Michael Dawson <michael_dawson@ca.ibm.com>
Author: Sampson Gao <sampsong@ca.ibm.com>
Author: Taylor Woll <taylor.woll@microsoft.com>
PR-URL: https://github.com/nodejs/node/pull/11975
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
2017-04-03 10:31:25 +02:00
Jason Ginchereau 58c38c2d2e src: fix TracingController cleanup
This fixes an incorrect deletion of the `TracingController` instance,
which in some environments could cause an error about an invalid
pointer passed to `free()`. The `TracingController` instance is
actually owned by a `unique_ptr` member of the platform, so calling
`platform::SetTracingController(nullptr)` is the correct way to
delete it. But before that, the `TraceBuffer` must be deleted in
order for the tracing loop to exit; that is accomplished by calling
`TracingController::Initialize(nullptr)`.

PR-URL: https://github.com/nodejs/node/pull/10623
Reviewed-By: Matthew Loring <mattloring@google.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
2017-01-07 13:56:43 +01:00
Jason Ginchereau 0f2f8efdde path: fallback to process cwd when resolving drive cwd
The `path.resolve()` function when given just a drive letter such as
"C:" tries to get a drive-specific CWD, but that isn't available in
cases when the process is not launched via cmd.exe and the process
CWD has not been explicitly set on that drive.

This change adds a fallback to the process CWD, if the process CWD
happens to be on the resolved drive letter. If the process CWD is on
another drive, then a drive-specific CWD cannot be resolved and
defaults to the drive's root as before.

Based on experimentation, the fixed behavior matches that of other
similar path resolution implementations on Windows I checked: .NET's
`System.IO.Path.GetFullPath()` and Python's `os.path.abspath()`.

In the automated path test cases the issue doesn't occur when the
tests are run normally from cmd.exe. But it did cause an assertion
when running the tests from PowerShell, that is fixed by this change.

PR-URL: https://github.com/nodejs/node/pull/8541
Fixes: https://github.com/nodejs/node/issues/7215
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2016-09-24 01:14:56 +03:00
Jason Ginchereau c1e47ed8c8 path: fix path.relative UNC path result
When the result of a path.relative() is an absolute UNC path, it should
include the leading backslashes.

Fixes: https://github.com/nodejs/node/issues/8444
PR-URL: https://github.com/nodejs/node/pull/8523
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
2016-09-20 09:56:50 -07:00