Skip to content

Conversation

Qard
Copy link
Member

@Qard Qard commented Nov 19, 2016

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

src, tools

Description of change

This is a backport of:

cjihrig and others added 5 commits November 18, 2016 19:03
This commit adds coverage for the timeout option used by
child_process exec() and execFile().

PR-URL: nodejs#9208
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
PR-URL: nodejs#9243
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
- Changed `assert.ok()` to `assert.strictEqual()`.
- Changed `var` to `const` where possible.

PR-URL: nodejs#9231
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ilkka Myller <[email protected]>
Writing data to TLSWrap instance during handshake will result in it
being queued in `write_item_queue_`. This queue won't get cleared up
until the end of the handshake.

Technically, it gets cleared on `~TLSWrap` invocation, however this
won't ever happen because every `WriteWrap` holds a reference to the
`TLSWrap` through JS object, meaning that they are doomed to be alive
for eternity.

To breach this dreadful contract a knight shall embark from the
`close` function to kill the dragon of memory leak with his magic
spear of `destroySSL`.

`destroySSL` cleans up `write_item_queue_` and frees `SSL` structure,
both are good for memory usage.

PR-URL: nodejs#9586
Reviewed-By: Ben Noordhuis <[email protected]>
Use zero-copy external string resources for storing the built-in JS
source code.  Saves a few hundred kilobyte of memory and consistently
speeds up `benchmark/misc/startup.js` by 2.5%.

Everything old is new again!  Commit 74954ce ("Add string class that
uses ExternalAsciiStringResource.") from 2011 did the same thing but
I removed that in 2013 in commit 34b0a36 ("src: don't use NewExternal()
with unaligned strings") because of a limitation in the V8 API.

V8 no longer requires that strings are aligned if they are one-byte
strings so it should be safe to re-enable external strings again.

PR-URL: nodejs#5458
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstaedt <[email protected]>
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. c++ Issues and PRs that require attention from people who are familiar with C++. v6.x labels Nov 19, 2016
@addaleax
Copy link
Member

Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for back-porting.

@MylesBorins
Copy link
Contributor

this will need to be rebased. @bnoordhuis how long should this back on v7.x before landing?

@Qard
Copy link
Member Author

Qard commented Nov 22, 2016

Ignore this PR. It cherry-picks cleanly without a backport PR.

@Qard Qard closed this Nov 22, 2016
@Qard Qard deleted the backport-5458-to-v6.x branch August 11, 2021 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants