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.

Sign up for GitHub

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

Jump to bottom

repl: Mitigate vm #548 function redefinition issue #7794

Closed
wants to merge 34 commits into from
Closed

repl: Mitigate vm #548 function redefinition issue #7794

wants to merge 34 commits into from

Conversation

princejwesley
Copy link
Contributor

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

repl

Description of change

Back port of #7624

node 🙈 ₹ git:(upstream ⚡ repl-tmp-548) ./node
> function name() { return "node"; };
undefined
> name()
'node'
> function name() { return "nodejs"; };
undefined
> name()
'nodejs'
>

mcollina and others added 30 commits July 15, 2016 08:18
@mcollina @evanlucas
win,msi: Added Italian translation
f25c7ce 
As titled. Tested by @piccoloaiutante.

PR-URL: nodejs#4647
Refs: wixtoolset/wix3#366
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Frederic Hemberger <mail@frederic-hemberger.de>
@Trott @evanlucas
tools: increase lint coverage
9db861b 
Extend linting to tools/license2rtf.js and any other JS that gets added
to the `tools` directory by default.

This incidentally simplifies lint invocation.

PR-URL: nodejs#7647
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
@addaleax @evanlucas
test,doc: clarify buf.indexOf(num) input range
25b3ff4 
Hopefully clarify the behaviour of `buffer.indexOf()` and
`buffer.includes()` for numbers in that they will be
truncated to uint8s.

Add tests for that behaviour.

Fixes: nodejs#7591
PR-URL: nodejs#7611
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@vsemozhetbyt @evanlucas
doc: fix typo in the CHANGELOG_V6
45a8fce 
compliment -> complement

PR-URL: nodejs#7568
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@silverwind @evanlucas
tools: consistent .eslintrc formatting
8bbb3eb 
All quotes in .eslintrc were unnecessary and inconsistently placed
across the file. Additionally, format the globals to be consistent
with the style of whitespace and sorted them alphabetically.

PR-URL: nodejs#7691
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
@pmq20 @evanlucas
win,msi: add zh-CN translations for the installer
e1e477e 
PR-URL: nodejs#2569
Reviewed-By: Jackson Tian <shvyo1987@gmail.com>
Reviewed-By: Wexpo Lyu <laosb@lns.unihubs.com>
Reviewed-By: Yiyu He <heyiyu.deadhorse@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
@evanlucas
inspector: Unify event queues
43b5bf4 
Current implementation tracks connected/disconnected status separately
which potentially introduces race condition.
This change introduces notion of session IDs and also posts
connect/disconnect events into the same queue as the messages. This way
Node knows what session given response belongs to and can discard
messages if the frontend for that session had disconnected.

This also fixes an issue when frontend was unable to attach to V8
instance that was running infinite loop.

PR-URL: nodejs#7271
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
@cjihrig @evanlucas
test: add common.rootDir
ccd4983 
A few of the child process tests can be simplified by computing
the OS specific root directory in common and then accessing that
value.

PR-URL: nodejs#7685
Reviewed-By: Roman Reiss <me@silverwind.io>
@claudiorodriguez @evanlucas
fs: rename event to eventType in fs.watch listener
f003465 
The name 'event' for the argument of the listener in
fs.watch was confusing considering FSWatcher also had
events. This changes the name of the argument to
eventType.

Fixes: nodejs#7504
PR-URL: nodejs#7506
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@cjihrig @evanlucas
test: cleanup IIFE tests
1af8c03 
A number of test files use IIFEs to separate distinct tests from
each other in the same file. The project has been moving toward
using block scopes and let/const in favor of IIFEs. This commit
moves IIFE tests to block scopes. Some additional cleanup such
as use of strictEqual() and common.mustCall() is also included.

PR-URL: nodejs#7694
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
@Trott @evanlucas
test: improve error message in test-tick-processor
d224b47 
Provide additional information about values that indicate test failed.

PR-URL: nodejs#7693
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
@jaimehrubiks @evanlucas
doc: removed old git conflict markers from fs.md
f0d9610 
"doc/api/fs.md" file had some conflict markers like "<<<<<<< HEAD"
that are visible at the bottom of https://nodejs.org/api/fs.html

PR-URL: nodejs#7590
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@RReverser @evanlucas
buffer: fix creating from zero-length ArrayBuffer
3b767b8 
Fixes regression where creating a new Buffer from an
empty ArrayBuffer would fail.

Ref: nodejs@85ab4a5
PR-URL: nodejs#7176
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
@bnoordhuis
deps: back-port d721121 from v8 upstream
c10ade9 
Original commit message:

    Quit creating array literal boilerplates from Crankshaft.

    It's such a corner case.

    BUG=

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

    Cr-Commit-Position: refs/heads/master@{nodejs#35346}

Fixes: nodejs#7454
PR-URL: nodejs#7633
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
@cjihrig @evanlucas
cluster: remove bind() and self
a855b30 
This commit removes the use of self and bind() from the cluster
module in favor of arrow functions.

PR-URL: nodejs#7710
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
@evanlucas
doc: fix typo in stream doc
46b9ef6 
Fix small typo in Buffering section of stream doc.
PR-URL: nodejs#7738
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@lance @evanlucas
doc: Warn against uncaughtException dependency.
c726ffb 
State in the documentation that `uncaughtException` is not a reliable
way to restart a crashed application, and clarify that an application
may crash in ways that do not trigger this event.

Use a documented synchronous function in example code.

Fixes: nodejs#6223

Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#6378
@whitlockjc @evanlucas
timers: fix processing of nested timers
9d9bd3c 
Whenever a timer is scheduled within another timer, there are a few
known issues that we are fixing:

* Whenever the timer being scheduled has the same timeout value as the
outer timer, the newly created timer can fire on the same tick of the
event loop instead of during the next tick of the event loop
* Whenever a timer is added in another timer's callback, its underlying
timer handle will be started with a timeout that is actually incorrect

This commit consists of
nodejs/node-v0.x-archive#17203 and
nodejs/node-v0.x-archive#25763.

Fixes: nodejs/node-v0.x-archive#9333
Fixes: nodejs/node-v0.x-archive#15447
Fixes: nodejs/node-v0.x-archive#25607
Fixes: nodejs#5426
PR-URL: nodejs#3063
@saadq @evanlucas
doc: fix inconsistencies in code style
669af6e 
Adds missing semicolons, removes extra white space, and properly indents
various code snippets in the documentation.

Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com>
PR-URL: nodejs#7745
@cjihrig @evanlucas
test: s/assert.fail/common.fail as appropriate
17591c3 
Many tests use assert.fail(null, null, msg) where it would be
simpler to use common.fail(msg). This is largely because
common.fail() is fairly new. This commit makes the replacement
when applicable.

PR-URL: nodejs#7735
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@indutny @evanlucas
deps: no /safeseh for ml64.exe
3bd40ff 
`ml64.exe` doesn't support `/safeseh` option. Do not attempt to use it
if `target_arch=="x64"`.

See: https://msdn.microsoft.com/en-us/library/s0ksfwcf.aspx
PR-URL: nodejs#7759
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
@addaleax @evanlucas
util: inspect boxed symbols like other primitives
60c459c 
Inspect boxed symbol objects in the same way other boxed primitives
are inspected.

Fixes: nodejs#7639
PR-URL: nodejs#7641
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@chjj @evanlucas
buffer: optimize hex_decode
ba6ab7c 
PR-URL: nodejs#7602
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@Trott @evanlucas
test: fix flaky test-http-server-consumed-timeout
aa045cd 
Using identical timeout values appears to have eliminated the flakiness
in the test.

Fixes: nodejs#7643
PR-URL: nodejs#7717
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
@italoacasas @evanlucas
doc: add added: information for stream
f7d3af6 
Ref: nodejs#6578
PR-URL: nodejs#7287
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
@bnoordhuis @evanlucas
src: fix handle leak in Buffer::New()
06bfb9e 
Fix handle leaks in Buffer::New() and Buffer::Copy() by creating the
handle scope before looking up the env with Environment::GetCurrent().

Environment::GetCurrent() calls v8::Isolate::GetCurrentContext(), which
creates a handle in the current scope, i.e., the scope created by the
caller of Buffer::New() or Buffer::Copy().

PR-URL: nodejs#7711
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@bnoordhuis @evanlucas
src: fix handle leak in BuildStatsObject()
978362d 
Create a handle scope before performing a check that creates a handle,
otherwise the handle is leaked into the handle scope of the caller.

PR-URL: nodejs#7711
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@bnoordhuis @evanlucas
src: fix handle leak in UDPWrap::Instantiate()
e46efd9 
Create a handle scope before performing a check that creates a handle,
otherwise the handle is leaked into the handle scope of the caller.

PR-URL: nodejs#7711
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@bnoordhuis @evanlucas
src: remove unnecessary HandleScopes
61d88d9 
API function callbacks run inside an implicit HandleScope.  We don't
need to explicitly create one and in fact introduce some unnecessary
overhead when we do.

PR-URL: nodejs#7711
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@khalsah @evanlucas
doc: correct sample output of buf.compare
62a3ff2 
Comparing the buffers `ABC` and `ABCD` returns `-1` not `1`.

PR-URL: nodejs#7777
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
gireeshpunathil and others added 4 commits July 19, 2016 05:45
@gireeshpunathil @evanlucas
test: avoid usage of mixed IPV6 addresses
9d59b7d 
The test case fails in AIX due to the mixed-use of unspecified
and loopback addresses. This is not a problem in most platforms
but fails in AIX. (In Windows too, but does not manifest as the
test is omitted in Windows for a different reason).

There exists no documented evidence which supports the mixed use
of unspecified and loopback addresses.

While AIX strictly follows the IPV6 specification with respect to
unspecified address ('::') and loopback address ('::1'), the test
case latches on to the behavior exhibited by other platforms,
and hence it fails in AIX.

The proposed fix is to make it work in all platforms including
AIX by using the loopback address for the client to connect,
as that is the address at which the server listens.

Fixes: nodejs#7563
PR-URL: nodejs#7702
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott @evanlucas
doc: update CTC governance information
059a721 
Update some outdated material. Provide some minor fixes. Wrap to 80
characters.

PR-URL: nodejs#7719
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@ofrobots @evanlucas
deps: v8_inspector no longer depends on wtf
f3182f6 
Remove wtf files as v8_inspector no longer needs them.

Ref: nodejs#7123

PR-URL: nodejs#7751
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
@princejwesley
repl: Mitigate vm nodejs#548 function redefinition issue
dedf6c4 
```js
node 🙈 ₹ git:(upstream ⚡ repl-tmp-548) ./node
> function name() { return "node"; };
undefined
> name()
'node'
> function name() { return "nodejs"; };
undefined
> name()
'nodejs'
>
```
@nodejs-github-bot nodejs-github-bot added the repl Issues and PRs related to the REPL subsystem. label Jul 19, 2016
@mscdex mscdex added the v6.x label Jul 19, 2016
@evanlucas
Copy link
Contributor

Thanks @princejwesley! CI: https://ci.nodejs.org/job/node-test-pull-request/3343/

Rubberstamp LGTM if CI is happy

@evanlucas
Copy link
Contributor

evanlucas commented Jul 20, 2016 •
edited
Loading

Hm, trying CI again (https://ci.nodejs.org/job/node-test-pull-request/3353/). I think there are some plinux issues on the ci right now. And don't worry about the conflicts showing here, I should be able to just cherry-pick that last commit now. I had to back some stuff out of v6.x

evanlucas pushed a commit that referenced this pull request Jul 21, 2016
@princejwesley @evanlucas
repl: Mitigate vm #548 function redefinition issue
72fc4eb 
```js
node 🙈 ₹ git:(upstream ⚡ repl-tmp-548) ./node
> function name() { return "node"; };
undefined
> name()
'node'
> function name() { return "nodejs"; };
undefined
> name()
'nodejs'
>
```

PR-URL: #7794
Reviewed-By: Evan Lucas <evanlucas@me.com>
@evanlucas
Copy link
Contributor

Landed in v6.x in 72fc4eb. Thanks!

@evanlucas evanlucas closed this Jul 21, 2016
evanlucas added a commit that referenced this pull request Jul 21, 2016
@evanlucas
2016-07-21, Version 6.3.1 (Current)
623bd46 
Notable changes:

* **buffer**:
  * Improve performance of Buffer.from(str, 'hex') and Buffer#write(str, 'hex'). (Christopher Jeffrey) #7602
  * Fix creating from zero-length ArrayBuffer. (Ingvar Stepanyan) #7176
* **deps**: Upgrade to V8 5.0.71.xx. (Ben Noordhuis) #7531
* **repl**: Fix issue with function redeclaration. (Prince J Wesley) #7794
* **util**: Fix inspecting of boxed symbols. (Anna Henningsen) #7641

PR-URL: #7782
@evanlucas evanlucas mentioned this pull request Jul 21, 2016
Merged
evanlucas added a commit that referenced this pull request Jul 21, 2016
@evanlucas
2016-07-21, Version 6.3.1 (Current)
f135a4c 
Notable changes:

* **buffer**:
  * Improve performance of Buffer.from(str, 'hex') and Buffer#write(str, 'hex'). (Christopher Jeffrey) #7602
  * Fix creating from zero-length ArrayBuffer. (Ingvar Stepanyan) #7176
* **deps**:
  * Upgrade to V8 5.0.71.xx. (Ben Noordhuis) #7531
  * Backport V8 instanceof bugfix (Franziska Hinkelmann) #7638
* **repl**: Fix issue with function redeclaration. (Prince J Wesley) #7794
* **util**: Fix inspecting of boxed symbols. (Anna Henningsen) #7641

PR-URL: #7782
evanlucas added a commit that referenced this pull request Jul 21, 2016
@evanlucas
2016-07-21, Version 6.3.1 (Current)
c21a212 
Notable changes:

* **buffer**:
  * Improve performance of Buffer.from(str, 'hex') and Buffer#write(str, 'hex'). (Christopher Jeffrey) #7602
  * Fix creating from zero-length ArrayBuffer. (Ingvar Stepanyan) #7176
* **deps**:
  * Upgrade to V8 5.0.71.xx. (Ben Noordhuis) #7531
  * Backport V8 instanceof bugfix (Franziska Hinkelmann) #7638
* **repl**: Fix issue with function redeclaration. (Prince J Wesley) #7794
* **util**: Fix inspecting of boxed symbols. (Anna Henningsen) #7641

PR-URL: #7782
@chrjean chrjean mentioned this pull request Jul 24, 2016
Merged
lukesampson pushed a commit to ScoopInstaller/Scoop that referenced this pull request Jul 24, 2016
@chrjean @lukesampson
Upgrade node to 6.3.1 (#966)
0aaa69d 
### Notable changes

* **buffer**:
  * Improve performance of Buffer.from(str, 'hex') and Buffer#write(str, 'hex'). (Christopher Jeffrey) [#7602](nodejs/node#7602)
  * Fix creating from zero-length ArrayBuffer. (Ingvar Stepanyan) [#7176](nodejs/node#7176)
* **deps**:
  * Upgrade to V8 5.0.71.xx. (Ben Noordhuis) [#7531](nodejs/node#7531)
  * Backport V8 instanceof bugfix (Franziska Hinkelmann) [#7638](nodejs/node#7638)
* **repl**: Fix issue with function redeclaration. (Prince J Wesley) [#7794](nodejs/node#7794)
* **util**: Fix inspecting of boxed symbols. (Anna Henningsen) [#7641](nodejs/node#7641)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.