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

src: Add missing using v8::MaybeLocal (v5.x) #5974

Closed
wants to merge 37 commits into from

Conversation

addaleax
Copy link
Member

Pull Request check-list

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to CONTRIBUTING.md?

Affected core subsystem(s)

src

Description of change

As of 2cbbaaf (#5756) there’s a missing using v8::MaybeLocal; in src/node.cc that breaks the build. This patch adds the necessary line.

I guess this slipped through because the corresponding line is present in master?

This test is failing btw.

/cc @evanlucas @trevnorris

JacksonTian and others added 30 commits March 30, 2016 14:06
When create Buffer from empty string will touch
C++ binding also.

This patch can improve edge case ~70% faster.

PR-URL: nodejs#4414
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Changes to Node core in order to allow compilation for linuxOne.

The ../archs/linux32-s390x/opensslconf.h and
../archs/linux64-s390x/opensslconf.h were automatically
generated by running make linux-ppc linux-ppc64 in the
deps/openssl/config directory as per our standard
practice

After these changes we still need a version of v8
which supports linuxOne but that will be coming soon
in the 5.1 version of v8.  Until then with these changes
we'll be able to create a hybrid build which pulls in
v8 from the http://github/andrewlow repo.

PR-URL: nodejs#5941
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
The create_android_makefiles script will create .mk files for node and
all of its dependencies ready to be build using Android build system.

Signed-off-by: Robert Chiras <[email protected]>
PR-URL: nodejs#5544
Reviewed-By: Ben Noordhuis <[email protected]>
Modified android-configure script to support also x86 arch.
Currently added support only for ia32 target arch.
Also, compile openssl without asm, since using the asm sources will make
node fail to run on Android, because it adds text relocations.

Signed-off-by: Robert Chiras <[email protected]>
PR-URL: nodejs#5544
Reviewed-By: Ben Noordhuis <[email protected]>
The socket list module (used by child_process) currently uses the
`var self = this;` pattern for context in several places, this PR
replaces this with arrow functions or passing a parameter in where
appropriate.

Note that the `var self = this` in the _request is intentioanlly
left in place since it is not trivial to refactor it and the current
pattern isn't bad given the use case.

PR-URL: nodejs#5860
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Brian White <[email protected]>
Original commit message:

    Unbreak --gdbjit for embedders.

    Embedders don't use d8.cc.  Move gdbjit initialization to api.cc.

    Review URL: https://codereview.chromium.org/1710253002

Fixes: nodejs#2076
PR-URL: nodejs#5577
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Currently we use `{}` for the `lookup` function to find the relevant
resolver to the dns.resolve function. It is preferable to use an
object without a Object.prototype, currently for example you can do
something like:

```js
dns.resolve("google.com", "toString", console.log);
```

And get `[Object undefined]` logged and the callback would never be
called. This is unexpected and strange behavior in my opinion.
In addition, if someone adds a property to `Object.prototype` might
also create unexpected results.

This pull request fixes it, with it an appropriate error is thrown.

PR-URL: nodejs#5843
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Refactor a forEach to a `map` in the `setServers` function of the
dns module - simplifying the code. In addition, use more descriptive
variable names and `const` over `var` where possible.

PR-URL: nodejs#5803
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
Update example of readInt32LE method. buf.readInt32LE(1) is supposed to
throw an error as it has only four elements and it tries to read 32
bits from three bytes.

Fixes: nodejs#5889
PR-URL: nodejs#5890
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
PR-URL: nodejs#5876
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Current html result of a list after heading is <div
class="signature"><ul>...</div></ul>. Correct it to <div
class="signature"><ul>...</ul></div>.

PR-URL: nodejs#5874
Fixes: nodejs#5873
Reviewed-By: Roman Reiss <[email protected]>
Implementing the suggestion in
nodejs#4554 this pull request renames
the parameter name in all the places that accept an event name as a parameter.

Previously, the parameter has been called `event` or `type`. Now as suggested
it is consistently called `eventName`.

PR-URL: nodejs#5850
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Explain the expected properties in path.format

Fixes: nodejs#5746
PR-URL: nodejs#5801
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Fixes a copy typo in the events.md docs.

PR-URL: nodejs#5849
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Event 9 must include the string terminator in the last descriptor.
Event 23 must be published with no descriptors, in accordance with
the manifest.

PR-URL: nodejs#5742
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
This uses libuv's mkdtemp function to provide a way to create a
temporary folder, using a prefix as the path. The prefix is appended
six random characters. The callback function will receive the name
of the folder that was created.

Usage example:

fs.mkdtemp('/tmp/foo-', function(err, folder) {
    console.log(folder);
        // Prints: /tmp/foo-Tedi42
});

The fs.mkdtempSync version is also provided. Usage example:

console.log(fs.mkdtemp('/tmp/foo-'));
    // Prints: tmp/foo-Tedi42

This pull request also includes the relevant documentation changes
and tests.

PR-URL: nodejs#5333
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
Reviewed-By: James M Snell <[email protected]>
The Regex implementation is not faster than ascii code compare.

the field name is shorter, the speed is faster.

benchmark result here:

https://bitbucket.org/snippets/JacksonTian/Rnbad/benchmark-result

PR-URL: nodejs#4790
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Brian White <[email protected]>
Only treat the gzip magic bytes, when encountered within the file
after reading a single block, as the start of a new member when
the previous member has ended.

Add test files that reliably reproduce nodejs#5852. The gzipped file
in test/fixtures/pseudo-multimember-gzip.gz contains the gzip
magic bytes exactly at the position that node encounters after having
read a single block, leading it to believe that a new data
member is starting.

Fixes: nodejs#5852
PR-URL: nodejs#5863
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Added build-addons task, it allows to build and test native addons
during test-ci task. Basically it should work in same way like
Makefile "build-addons" task.

Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: João Reis <[email protected]>
PR-URL: nodejs#5886
Fixes: nodejs#2537
Current processList function in tools/doc/json.js does not recognise
{"type":"loose_item_start"}. Fix it.

PR-URL: nodejs#5943
Fixes: nodejs#5942
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Robert Lindstädt <[email protected]>
Added safe internal references for 'clearTimeout(..)', 'active(..)', and
'unenroll(..)'. Changed various API refs from 'export.*' to use these
safe internal references.

Now, overwriting the global API identifiers does not create potential
breakage and/or race conditions. See Issue nodejs#2493.

PR-URL: nodejs#5882
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Fixes: nodejs#2493
Fix long-broken test-debugger-client by adding missing `\r\n\r\n`
separator.

PR-URL: nodejs#5851
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Increase timeout on Raspberry Pi to alleviate flakiness.

Fixes: nodejs#5854
PR-URL: nodejs#5856
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
parallel/test-dns-cares-domains needs a working internet connection
to function (or a local DNS resolver that returns an answer quickly),
otherwise it times out.  Move it to test/internet.

PR-URL: nodejs#5905
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Fixes: nodejs#5892
PR-URL: nodejs#5902
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#5882
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
As per nodejs#5085
exclude new test from AIX until we have fixes for
libuv for fs watching on AIX.  Excluding test
so AIX tests are green and we don't miss
other regressions

PR-URL: nodejs#5937
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
There were 2 tests using curl:

`test-http-304.js` is removed because it was initially included to test
that the 304 response does not contain a body, and this is already
covered by `test-http-chunked-304.js`.

`test-http-curl-chunk-problem` has been renamed and refactored so
instead of using curl, it uses 2 child node processes: one for sending
the HTTP request and the other to calculate the sha1sum. Originally,
this test was introduced to fix a bug in `[email protected]`, and it was not
fixed until `[email protected]`. A modified version of this test has been run
with `[email protected]` and reproduces the problem. This same test has been
run with `[email protected]` and runs correctly.

Fixes: nodejs#5174
PR-URL: nodejs#5750
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Check that piping a large chunk of data from `process.stdin`
into `process.stdout` does not lose any data by verifying that
the output has the same size as the input.

This is a regression test for nodejs#5927 and fails for the commits
in the range [ace1009..89abe86).

PR-URL: nodejs#5949
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
tomgco and others added 7 commits March 30, 2016 14:15
Based on the conversation in nodejs#4243 this implements a way to increase
and decrease the size of the thread pool used in v8.

Currently v8 restricts the thread pool size to `kMaxThreadPoolSize`
which at this commit is (4). So it is only possible to
decrease the thread pool size at the time of this commit. However with
changes upstream this could change at a later date.
If set to 0 then v8 would choose an appropriate size of the thread pool
based on the number of online processors.

PR-URL: nodejs#4344
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Enable support for standalone block statements.

```js
node 🙈 ₹ git:(upstream ⚡ bare-block) ./node
> { var x = 3; console.log(x); }
3
undefined
> {}
{}
> { x:1, y:"why not", z: function() {} }
{ x: 1, y: 'why not', z: [Function] }
>
```
For the ambiguous inputs like `{ x }`, the existing REPL
behaviour (ES6 literal shorthand) is preserved (prefers
expression over statement).

Fixes: nodejs#5576
PR-URL: nodejs#5581
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Fixes: nodejs#3702
PR-URL: nodejs#5858
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Previously, we emitted ip and addressType. This change includes the host
as the last argument to the lookup event.

PR-URL: nodejs#5598
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
reduce using RegExp for string test. This pull reuqest replaces
various usages of regular expressions in favor of the ES2015
startsWith and endsWith methods.

PR-URL: nodejs#5753
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Brian White <[email protected]>
As of 2cbbaaf (nodejs#5756) there’s a missing
`using v8::MaybeLocal;` in `src/node.cc` that breaks the build.
This patch adds the necessary line.
@cjihrig
Copy link
Contributor

cjihrig commented Mar 31, 2016

LGTM

@mscdex mscdex added lib / src Issues and PRs related to general changes in the lib or src directory. v5.x labels Mar 31, 2016
@evanlucas
Copy link
Contributor

LGTM

evanlucas pushed a commit that referenced this pull request Mar 31, 2016
As of 2cbbaaf (#5756) there’s a missing
`using v8::MaybeLocal;` in `src/node.cc` that breaks the build.
This patch adds the necessary line.

PR-URL: #5974
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
@evanlucas
Copy link
Contributor

Landed in v5.x in d044898

Thanks!

@evanlucas evanlucas closed this Mar 31, 2016
@addaleax addaleax deleted the fix-using-maybelocal-on-5.x branch March 31, 2016 14:52
evanlucas pushed a commit that referenced this pull request Mar 31, 2016
As of 2cbbaaf (#5756) there’s a missing
`using v8::MaybeLocal;` in `src/node.cc` that breaks the build.
This patch adds the necessary line.

PR-URL: #5974
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.