From 4f1c82f995ec2746644b0195f529d85d25151c3c Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 12 Apr 2016 12:30:03 +0200 Subject: [PATCH 1/3] streams: support unlimited synchronous cork/uncork cycles net streams can request multiple chunks to be written in a synchronous fashion. If this is combined with cork/uncork, en error is currently thrown because of a regression introduced in: https://github.com/nodejs/node/commit/89aeab901ac9e34c79be3854f1aa41f2a4fb6888 (https://github.com/nodejs/node/pull/4354). Fixes: https://github.com/nodejs/node/issues/6154 PR-URL: https://github.com/nodejs/node/pull/6164 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Mathias Buus Reviewed-By: James M Snell --- lib/_stream_writable.js | 15 ++++++----- test/parallel/test-net-sync-cork.js | 41 +++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 6 deletions(-) create mode 100644 test/parallel/test-net-sync-cork.js diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js index 7bbb97b16aa415..8b2e90ca43c555 100644 --- a/lib/_stream_writable.js +++ b/lib/_stream_writable.js @@ -112,10 +112,9 @@ function WritableState(options, stream) { // count buffered requests this.bufferedRequestCount = 0; - // create the two objects needed to store the corked requests - // they are not a linked list, as no new elements are inserted in there + // allocate the first CorkedRequest, there is always + // one allocated and free to use, and we maintain at most two this.corkedRequestsFree = new CorkedRequest(this); - this.corkedRequestsFree.next = new CorkedRequest(this); } WritableState.prototype.getBuffer = function writableStateGetBuffer() { @@ -387,12 +386,16 @@ function clearBuffer(stream, state) { doWrite(stream, state, true, state.length, buffer, '', holder.finish); - // doWrite is always async, defer these to save a bit of time + // doWrite is almost always async, defer these to save a bit of time // as the hot path ends with doWrite state.pendingcb++; state.lastBufferedRequest = null; - state.corkedRequestsFree = holder.next; - holder.next = null; + if (holder.next) { + state.corkedRequestsFree = holder.next; + holder.next = null; + } else { + state.corkedRequestsFree = new CorkedRequest(state); + } } else { // Slow case, write chunks one-by-one while (entry) { diff --git a/test/parallel/test-net-sync-cork.js b/test/parallel/test-net-sync-cork.js new file mode 100644 index 00000000000000..a35a74a82153b1 --- /dev/null +++ b/test/parallel/test-net-sync-cork.js @@ -0,0 +1,41 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); + +const server = net.createServer(handle); + +const N = 100; +const buf = Buffer('aa'); + +server.listen(common.PORT, function() { + const conn = net.connect(common.PORT); + + conn.on('connect', () => { + let res = true; + let i = 0; + for (; i < N && res; i++) { + conn.cork(); + conn.write(buf); + res = conn.write(buf); + conn.uncork(); + } + assert.equal(i, N); + conn.end(); + }); +}); + +process.on('exit', function() { + assert.equal(server.connections, 0); +}); + +function handle(socket) { + socket.resume(); + + socket.on('error', function(err) { + socket.destroy(); + }).on('close', function() { + server.close(); + }); +} From f46952e72747c8cca846a01fadfd9bddb630a55f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A1=D0=BA=D0=BE=D0=B2=D0=BE=D1=80=D0=BE=D0=B4=D0=B0=20?= =?UTF-8?q?=D0=9D=D0=B8=D0=BA=D0=B8=D1=82=D0=B0=20=D0=90=D0=BD=D0=B4=D1=80?= =?UTF-8?q?=D0=B5=D0=B5=D0=B2=D0=B8=D1=87?= Date: Mon, 25 Apr 2016 21:56:19 +0300 Subject: [PATCH 2/3] buffer: safeguard against accidental kNoZeroFill This makes sure that `kNoZeroFill` flag is not accidentally set by moving the all the flag operations directly inside `createBuffer()`. It safeguards against logical errors like https://github.com/nodejs/node/issues/6006. This also ensures that `kNoZeroFill` flag is always restored to 0 using a try-finally block, as it could be not restored to 0 in cases of failed or zero-size `Uint8Array` allocation. It safeguards against errors like https://github.com/nodejs/node/issues/2930. It also makes the `size > 0` check not needed there. PR-URL: https://github.com/nodejs/node-private/pull/30 Reviewed-By: James M Snell Reviewed-By: Trevor Norris --- lib/buffer.js | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index a24bdd0e006f9f..eff91be2882630 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -19,17 +19,20 @@ binding.setupBufferJS(Buffer.prototype, bindingObj); const flags = bindingObj.flags; const kNoZeroFill = 0; -function createBuffer(size) { - const ui8 = new Uint8Array(size); - Object.setPrototypeOf(ui8, Buffer.prototype); - return ui8; +function createBuffer(size, noZeroFill) { + flags[kNoZeroFill] = noZeroFill ? 1 : 0; + try { + const ui8 = new Uint8Array(size); + Object.setPrototypeOf(ui8, Buffer.prototype); + return ui8; + } finally { + flags[kNoZeroFill] = 0; + } } function createPool() { poolSize = Buffer.poolSize; - if (poolSize > 0) - flags[kNoZeroFill] = 1; - allocPool = createBuffer(poolSize); + allocPool = createBuffer(poolSize, true); poolOffset = 0; } createPool(); @@ -65,13 +68,10 @@ function Buffer(arg, encoding) { Object.setPrototypeOf(Buffer.prototype, Uint8Array.prototype); Object.setPrototypeOf(Buffer, Uint8Array); - function SlowBuffer(length) { if (+length != length) length = 0; - if (length > 0) - flags[kNoZeroFill] = 1; - return createBuffer(+length); + return createBuffer(+length, true); } Object.setPrototypeOf(SlowBuffer.prototype, Uint8Array.prototype); @@ -93,9 +93,7 @@ function allocate(size) { // Even though this is checked above, the conditional is a safety net and // sanity check to prevent any subsequent typed array allocation from not // being zero filled. - if (size > 0) - flags[kNoZeroFill] = 1; - return createBuffer(size); + return createBuffer(size, true); } } From 9cf628eae3a9b6969eca0d82c6bb0bcb18552867 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Wed, 4 May 2016 11:16:40 -0700 Subject: [PATCH 3/3] 2016-05-05, Version 4.4.4 'Argon' (LTS) Notable changes * deps: * update openssl to 1.0.2h. (Shigeki Ohtsu) [#6551](https://github.com/nodejs/node/pull/6551) - Please see our blog postfor more info on the security contents of this release. https://nodejs.org/en/blog/vulnerability/openssl-may-2016/ --- CHANGELOG.md | 20 ++++++++++++++++++++ src/node_version.h | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 702b232a138005..5061c9074e8d9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,25 @@ # Node.js ChangeLog +## 2016-05-05, Version 4.4.4 'Argon' (LTS), @thealphanerd + +### Notable changes + +* **deps**: + * update openssl to 1.0.2h. (Shigeki Ohtsu) [#6551](https://github.com/nodejs/node/pull/6551) + - Please see our [blog post](https://nodejs.org/en/blog/vulnerability/openssl-may-2016/) for more info on the security contents of this release. + +### Commits + +* [[`f46952e727`](https://github.com/nodejs/node/commit/f46952e727)] - **buffer**: safeguard against accidental kNoZeroFill (Сковорода Никита Андреевич) [nodejs/node-private#30](https://github.com/nodejs/node-private/pull/30) +* [[`4f1c82f995`](https://github.com/nodejs/node/commit/4f1c82f995)] - **streams**: support unlimited synchronous cork/uncork cycles (Matteo Collina) [#6164](https://github.com/nodejs/node/pull/6164) +* [[`1efd96c767`](https://github.com/nodejs/node/commit/1efd96c767)] - **deps**: update openssl asm and asm_obsolete files (Shigeki Ohtsu) [#6551](https://github.com/nodejs/node/pull/6551) +* [[`c450f4a293`](https://github.com/nodejs/node/commit/c450f4a293)] - **deps**: add -no_rand_screen to openssl s_client (Shigeki Ohtsu) [nodejs/io.js#1836](https://github.com/nodejs/io.js/pull/1836) +* [[`baedfbae6a`](https://github.com/nodejs/node/commit/baedfbae6a)] - **openssl**: fix keypress requirement in apps on win32 (Shigeki Ohtsu) [iojs/io.js#1389](https://github.com/iojs/io.js/pull/1389) +* [[`ff3045e40b`](https://github.com/nodejs/node/commit/ff3045e40b)] - **deps**: fix asm build error of openssl in x86_win32 (Shigeki Ohtsu) [iojs/io.js#1389](https://github.com/iojs/io.js/pull/1389) +* [[`dc8dc97db3`](https://github.com/nodejs/node/commit/dc8dc97db3)] - **deps**: fix openssl assembly error on ia32 win32 (Fedor Indutny) [iojs/io.js#1389](https://github.com/iojs/io.js/pull/1389) +* [[`2dfeb01213`](https://github.com/nodejs/node/commit/2dfeb01213)] - **deps**: copy all openssl header files to include dir (Shigeki Ohtsu) [#6551](https://github.com/nodejs/node/pull/6551) +* [[`72f9952516`](https://github.com/nodejs/node/commit/72f9952516)] - **deps**: upgrade openssl sources to 1.0.2h (Shigeki Ohtsu) [#6551](https://github.com/nodejs/node/pull/6551) + ## 2016-04-12, Version 4.4.3 'Argon' (LTS), @thealphanerd ### Notable Changes diff --git a/src/node_version.h b/src/node_version.h index 0c852b10e66653..3b0697fda47f0f 100644 --- a/src/node_version.h +++ b/src/node_version.h @@ -8,7 +8,7 @@ #define NODE_VERSION_IS_LTS 1 #define NODE_VERSION_LTS_CODENAME "Argon" -#define NODE_VERSION_IS_RELEASE 0 +#define NODE_VERSION_IS_RELEASE 1 #ifndef NODE_STRINGIFY #define NODE_STRINGIFY(n) NODE_STRINGIFY_HELPER(n)