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

198 per module environment #208

Conversation

gabrielschulhof
Copy link
Collaborator

This is an alternative to #206. It doesn't tack the napi_env onto any global node object, but rather creates a new one during module registration, and then passes it around, including into the context for all callbacks/accessors/finalizers.

fhinkel and others added 30 commits March 19, 2017 21:41
Original commit message:
  [debugger] fix switch block source positions.

  The switch statement itself is part of the switch block.
  However, the source position of the statement is outside of
  the block. This leads to confusion for the debugger, if the
  switch block pushes a block context: the current context is
  a block context, but the scope analysis based on the current
  source position tells the debugger that we should be outside
  the scope, so we should have the function context.

  [email protected]
  BUG=v8:6085
  Review-Url: https://codereview.chromium.org/2744213003
  Cr-Commit-Position: refs/heads/master@{#43744}
  Committed: https://chromium.googlesource.com/v8/v8/+/09de9969ccb9bc3bbd667788afad665b309d02f5

Fixes: nodejs/node#11746
PR-URL: nodejs/node#11905
Fixes: nodejs/node#11746
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
PR-URL: nodejs/node#11932
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Prototypes are not strict equal when they are from
different contexts. Therefore, assert.strictEqual()
fails for objects that are created in different
contexts, e.g., in vm.runInContext().

Instead of expecting the prototypes to be equal,
only check the properties of the objects
for equality.

PR-URL: nodejs/node#11862
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
OnRead() calls into JS land which can result in the SSL context object
being destroyed on return.  Check that `ssl_ != nullptr` afterwards.

Fixes: nodejs/node#11885
PR-URL: nodejs/node#11898
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Check the options passed to Socket.prototype.connect() to
validate the type of the lookup property.

PR-URL: nodejs/node#11873
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
This test checks for the different input types for the generation
of UNIX timestamps in the fs module.

PR-URL: nodejs/node#11886
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs/node#11872
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Since file URLs can not have `username/password/port`,
the specification was updated to restrict setting protocol to "file".

Refs: whatwg/url#269
Fixes: nodejs/node#11785
PR-URL: nodejs/node#11887
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Synchronize url-setter-test to upstream.

Refs: web-platform-tests/wpt#5112
PR-URL: nodejs/node#11887
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
PR-URL: nodejs/node#11914
Fixes: nodejs/node#11913
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Check that stdin, stdout and stderr are valid file descriptors on
Windows. If not, reopen them with 'nul' file.

Refs: nodejs/node#875
Fixes: nodejs/node#11656
PR-URL: nodejs/node#11863
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
GitHub now renders Markdown in CommonMark where spaces in a link
destination are invalid and need to be escaped.

PR-URL: nodejs/node#11944
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
PR-URL: nodejs/node#11594
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
PR-URL: nodejs/node#11594
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Where inclusion of a lengthy URL causes a line to exceed 80 characters
in our code base, do not report the line length as a linting error.

PR-URL: nodejs/node#11890
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
lib/buffer.js uses a function declaration for `Buffer`. So it never
uses an instance of `Buffer` in the global scope. Therefore the
disabling of the `require-buffer` custom rule is not needed. Remove the
comment.

PR-URL: nodejs/node#11906
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Currently test-tls-socket-close will fail if node
was built using --without-ssl. This commit adds a check to
verify is crypto support exists and if not skip this test.

PR-URL: nodejs/node#11911
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
This removes common.v8ForceOptimization calls from url and vm benchmark
files.

PR-URL: nodejs/node#11908
Fixes: nodejs/node#11895
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Make it call-site-cwd-independent.

PR-URL: nodejs/node#11904
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Notable Changes:

* buffer:
  - The performance of `.toJSON()` is now up to 2859% faster on average
    (Brian White) nodejs/node#10895

* IPC:
  - Batched writes have been enabled for process IPC on platforms that
    support Unix Domain Sockets. (Alexey Orlenko)
    nodejs/node#10677
  - Performance gains may be up to 40% for some workloads.

* http:
  - Control characters are now always rejected when using
    `http.request()`. (Ben Noordhuis)
    nodejs/node#8923

* node:
  - Heap statistics now support values larger than 4GB. (Ben Noordhuis)
    nodejs/node#10186

PR-URL: nodejs/node#11760
Notable changes

* performance: The performance of several APIs has been improved.
  - `Buffer.compare()` is up to 35% faster on average. (Brian White)
    nodejs/node#10927
  - `buffer.toJSON()` is up to 2859% faster on average. (Brian White)
    nodejs/node#10895
  - `fs.*statSync()` functions are now up to 9.3% faster on average.
    (Brian White) nodejs/node#11522
  - `os.loadavg` is up to 151% faster. (Brian White)
    nodejs/node#11516
  - `process.memoryUsage()` is up to 34% faster. (Brian White)
    nodejs/node#11497
  - `querystring.unescape()` for `Buffer`s is 15% faster on average.
    (Brian White) nodejs/node#10837
  - `querystring.stringify()` is up to 7.8% faster on average.
    (Brian White) nodejs/node#10852
  - `querystring.parse()` is up to 21% faster on average. (Brian White)
    nodejs/node#10874

* IPC:
  - Batched writes have been enabled for process IPC on platforms that
    support Unix Domain Sockets. (Alexey Orlenko)
    nodejs/node#10677
  - Performance gains may be up to 40% for some workloads.

* child_process:
  - `spawnSync` now returns a null `status` when child is terminated by
    a signal. (cjihrig) nodejs/node#11288
  - This fixes the behavior to act like `spawn()` does.

* http:
  - Control characters are now always rejected when using
    `http.request()`. (Ben Noordhuis)
    nodejs/node#8923
  - Debug messages have been added for cases when headers contain
    invalid values. (Evan Lucas)
    nodejs/node#9195

* node:
  - Heap statistics now support values larger than 4GB. (Ben Noordhuis)
    nodejs/node#10186

* timers:
  - Timer callbacks now always maintain order when interacting with
    domain error handling. (John Barboza)
    nodejs/node#10522

PR-URL: nodejs/node#11759
Notable changes:

* deps: Add node-inspect 1.10.6 (Jan Krems) nodejs/node#11869
* inspector: proper WS URLs when bound to 0.0.0.0 (Eugene Ostroukhov) nodejs/node#11850
* tls: fix segfault on destroy after partial read. (Ben Noordhuis) nodejs/node#11898

PR-URL: nodejs/node#11941
The bundled c-ares isn't very suitable for consumption by addons,
isn't kept stable, and isn't exported on windows.

PR-URL: nodejs/node#10283
Refs: nodejs/node-gyp#1055
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs/node#11858
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
The entire `URLSearchParams` class is now fully spec-compliant.

PR-URL: nodejs/node#11858
Fixes: nodejs/node#10821
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
It only returns 0, nor is it likely to have any error conditions in the
future.

PR-URL: nodejs/node#11922
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Replace timer/timeout race with event-based ordering, eliminating test
flakiness.

PR-URL: nodejs/node#11921
Fixes: nodejs/node#11912
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
While `child_process.execFile()` gets called in places in the test
suite, there are no explicit test for it and there are parts of the
implementation that are not covered by tests. This adds a minimal test
that increases (but does not complete) coverage for the implementation.

PR-URL: nodejs/node#11929
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Also add test cases for partial writes and invalid indices.

PR-URL: nodejs/node#11927
Fixes: nodejs/node#8724
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Add a space for minimal readability.

PR-URL: nodejs/node#11925
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax and others added 3 commits March 31, 2017 22:12
Define `V8_ENABLE_CHECKS` in `common.gypi` for the debug mode.
Without this, these checks would only be present in the object files
generated from the V8 build, and so for inline functions in v8.h
multiple different definitions could be generated, where one definition
includes the check and the other does not.

Refs: nodejs/node#11975 (comment)
PR-URL: nodejs/node#12029
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jason Ginchereau <[email protected]>
Reviewed-By: Hitesh Kanwathirtha <[email protected]>
Adds support for the PSS padding scheme. Until now, the sign/verify
functions used the old EVP_Sign*/EVP_Verify* OpenSSL API, making it
impossible to change the padding scheme. Fixed by first computing the
message digest and then signing/verifying with a custom EVP_PKEY_CTX,
allowing us to specify options such as the padding scheme and the PSS
salt length.

Fixes: nodejs/node#1127
PR-URL: nodejs/node#11705
Reviewed-By: Shigeki Ohtsu <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
* Rename listen to listenInCluster
* Rename _listen2 to _setupListenHandle
* Remove _listen since it's a one-liner only used in one place
* Correct comments in server.listen

PR-URL: nodejs/node#11796
Reviewed-By: James M Snell <[email protected]>
addaleax

This comment was marked as off-topic.

Adds a centered logo to the README to make it a little more festive. As
centering is not possible in pure Markdown, a bit of HTML is used.

PR-URL: nodejs/node#12148
Ref: nodejs/node#6920
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@gabrielschulhof
Copy link
Collaborator Author

gabrielschulhof commented Apr 2, 2017

@jasongin we need to abort when

  1. We have NAPI_PREAMBLE() because it may need to store the last exception, which is stored in the env.
  2. We have NAPI_ARG(env, <anything>) because it needs to set the last error if <anything> is null, and the last error is stored in the env.

Really, the only time we really wouldn't need the environment is when a function did not need anything stored inside it, and always returned napi_ok. The closest function we have to that is napi_typeof(), but since it needs to make sure that result is not null, it has a NAPI_ARG() for result, which needs the env.

I've gone through the entire src/node_api.cc file and for all public APIs we have one of the above two conditions. So, I think we need to keep the abort, unless we are prepared to accept that napi_get_last_error_info() will sometimes not return the last error, because when the last error happened, then env was not passed in, so it couldn't be recorded, and thus, if env is around, after all, just not in that one call, then what you get from napi_get_last_error_info() is the second-last error, not the last error.

@gabrielschulhof
Copy link
Collaborator Author

@jasongin if people are more likely to use the C++ wrapper anyway, since it looks a lot like NAN, then we need not worry about the abort so much, because the wrapper will always correctly pass in the env and the users of the wrapper need not worry about it.

@jasongin
Copy link
Member

jasongin commented Apr 3, 2017

unless we are prepared to accept that napi_get_last_error_info() will sometimes not return the last error, because when the last error happened, then env was not passed in, so it couldn't be recorded, and thus, if env is around, after all, just not in that one call, then what you get from napi_get_last_error_info() is the second-last error, not the last error.

That's actually not a problem: with this change, napi_get_last_error_info() is defined as returning the last error for the env that you give it. So if you give it a null env, then it should return napi_invalid_arg, which would be the same as the last error for any other call that was attempted using a null env. There is no inconsistency.

So I still don't think aborting is the best response to a null env. But this is such a minor thing that hardly anyone will ever encounter, that I don't feel strongly about it.

gabrielschulhof pushed a commit to gabrielschulhof/abi-stable-node that referenced this pull request Apr 3, 2017
1. We define struct napi_env__ to include the isolate, the last
exception, and the info about the last error.

2. We instantiate one struct napi_env__ during module registration and
we pass it into the FunctionCallbackInfo for all subsequent entries into
N-API when we create functions/accessors/finalizers.

Once module unloading will be supported we shall have to delete the
napi_env we create during module init.

There is a clear separation between public and private API wrt. env:

1. Public APIs assert that env is not nullptr as their first action.

2. Private APIs need not validate env. They assume it's not nullptr.

Fixes nodejs#198
Closes nodejs#208
gabrielschulhof pushed a commit to gabrielschulhof/abi-stable-node that referenced this pull request Apr 3, 2017
1. We define struct napi_env__ to include the isolate, the last
exception, and the info about the last error.

2. We instantiate one struct napi_env__ during module registration and
we pass it into the FunctionCallbackInfo for all subsequent entries into
N-API when we create functions/accessors/finalizers.

Once module unloading will be supported we shall have to delete the
napi_env we create during module init.

There is a clear separation between public and private API wrt. env:

1. Public APIs assert that env is not nullptr as their first action.

2. Private APIs need not validate env. They assume it's not nullptr.

Fixes nodejs#198
Closes nodejs#208
TimothyGu and others added 12 commits April 3, 2017 09:50
Also support Uint8Array as a `dictionary` option.

PR-URL: nodejs/node#12001
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Display `v8::External` values as `[External]` rather than `{}`
which makes them look like objects.

PR-URL: nodejs/node#12151
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
assert.deepEqual and assert.deepStrictEqual currently return true for
any pair of Maps and Sets regardless of content. This patch adds
support in deepEqual and deepStrictEqual to verify the contents of Maps
and Sets.

Deeo equivalence checking is currently an
O(n^2) operation, and worse, it gets slower exponentially if maps
and sets were nested.

Note that this change breaks compatibility with previous versions of
deepEqual and deepStrictEqual if consumers were depending on all maps
and sets to be seen as equivalent. The old behaviour was never
documented, but nevertheless there are certainly some tests out there
which depend on it.

Support has stalled because the assert API was frozen, but was recently
unfrozen in CTC#63.

---

Later squashed in:

This change updates the checks for deep equality checking on Map and Set
to check all set values / all map keys to see if any of them match the
expected result.

This change is much slower, but based on the conversation in the pull
request its probably the right approach.

Fixes: nodejs/node#2309
Refs: tape-testing/tape#342
Refs: nodejs/node#2315
Refs: nodejs/CTC#63
PR-URL: nodejs/node#12142
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
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 <[email protected]>
Author: Gabriel Schulhof <[email protected]>
Author: Hitesh Kanwathirtha <[email protected]>
Author: Ian Halliday <[email protected]>
Author: Jason Ginchereau <[email protected]>
Author: Michael Dawson <[email protected]>
Author: Sampson Gao <[email protected]>
Author: Taylor Woll <[email protected]>
PR-URL: nodejs/node#11975
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Fix the offset calculation for deserializing TypedArrays that are
not aligned in their original buffer.

Since `byteOffset` refers to the offset into the source `Buffer`
instance, not its underlying `ArrayBuffer`, that is what should
be passed to `buffer.copy`.

PR-URL: nodejs/node#12143
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
This changes to the way path parsing for non-special URLs.
It allows paths to be empty for non-special URLs and also
takes that into account when serializing.

Fixes: nodejs/node#11962
Refs: whatwg/url#213
PR-URL: nodejs/node#12058
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Refs: web-platform-tests/wpt#4586
Refs: nodejs/node#11887
PR-URL: nodejs/node#12058
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Updates:
+ Bring tests url-setter-tests from WPT, and put it as JavaScript
+ Comment out unpassed tests

Refs: web-platform-tests/wpt#5112
Refs: nodejs/node#11887
PR-URL: nodejs/node#12058
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Ref: nodejs/node#12143 (comment)
PR-URL: nodejs/node#12186
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
`Error[CODE]` becomes `Error [CODE]`

PR-URL: nodejs/node#12099
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
PR-URL: nodejs/node#12141
Ref: nodejs/CTC#89
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Prince John Wesley <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
makeCallback and makeStatsCallback are both tested intedependently.

PR-URL: nodejs/node#12140
Fixes: nodejs/node#12136
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@gabrielschulhof gabrielschulhof force-pushed the 198-per-module-environment branch 2 times, most recently from 8e34f29 to 36f60eb Compare April 3, 2017 20:40
1. We define struct napi_env__ to include the isolate, the last
exception, and the info about the last error.

2. We instantiate one struct napi_env__ during module registration and
we pass it into the FunctionCallbackInfo for all subsequent entries into
N-API when we create functions/accessors/finalizers.

Once module unloading will be supported we shall have to delete the
napi_env we create during module init.

There is a clear separation between public and private API wrt. env:

1. Public APIs assert that env is not nullptr as their first action.

2. Private APIs need not validate env. They assume it's not nullptr.

Fixes: nodejs#198
@gabrielschulhof gabrielschulhof deleted the 198-per-module-environment branch April 5, 2017 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.