From eb19df60f3b0ae25dc7194d2258c2994383b79b4 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 19 Apr 2016 14:46:53 -0400 Subject: [PATCH 1/6] stream_base: expose `bytesRead` getter This will provide `bytesRead` data on consumed sockets. Fix: #3021 --- lib/net.js | 14 ++++++---- src/env.h | 1 + src/stream_base-inl.h | 16 ++++++++++++ src/stream_base.h | 13 ++++++++-- test/parallel/test-net-bytes-read.js | 38 ++++++++++++++++++++++++++++ 5 files changed, 75 insertions(+), 7 deletions(-) create mode 100644 test/parallel/test-net-bytes-read.js diff --git a/lib/net.js b/lib/net.js index 61ff7327fbcec4..805e878bf45451 100644 --- a/lib/net.js +++ b/lib/net.js @@ -97,7 +97,6 @@ exports._normalizeConnectArgs = normalizeConnectArgs; // called when creating new Socket, or when re-using a closed Socket function initSocketHandle(self) { self.destroyed = false; - self.bytesRead = 0; self._bytesDispatched = 0; self._sockname = null; @@ -179,6 +178,9 @@ function Socket(options) { // Reserve properties this.server = null; this._server = null; + + // Used after `.destroy()` + this._bytesRead = 0; } util.inherits(Socket, stream.Duplex); @@ -470,6 +472,9 @@ Socket.prototype._destroy = function(exception, cb) { if (this !== process.stderr) debug('close handle'); var isException = exception ? true : false; + // `bytesRead` should be accessible after `.destroy()` + this._bytesRead = this._handle.bytesRead; + this._handle.close(() => { debug('emit close'); this.emit('close', isException); @@ -521,10 +526,6 @@ function onread(nread, buffer) { // will prevent this from being called again until _read() gets // called again. - // if it's not enough data, we'll just call handle.readStart() - // again right away. - self.bytesRead += nread; - // Optimization: emit the original buffer with end points var ret = self.push(buffer); @@ -580,6 +581,9 @@ Socket.prototype._getpeername = function() { return this._peername; }; +Socket.prototype.__defineGetter__('bytesRead', function() { + return this._handle ? this._handle.bytesRead : this._bytesRead; +}); Socket.prototype.__defineGetter__('remoteAddress', function() { return this._getpeername().address; diff --git a/src/env.h b/src/env.h index 9b117e1de0e391..afbade5dd81e70 100644 --- a/src/env.h +++ b/src/env.h @@ -72,6 +72,7 @@ namespace node { V(buffer_string, "buffer") \ V(bytes_string, "bytes") \ V(bytes_parsed_string, "bytesParsed") \ + V(bytes_read_string, "bytesRead") \ V(cached_data_string, "cachedData") \ V(cached_data_produced_string, "cachedDataProduced") \ V(cached_data_rejected_string, "cachedDataRejected") \ diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index 81114a265e9a08..522ba0ed7d3ba5 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -43,6 +43,13 @@ void StreamBase::AddMethods(Environment* env, v8::DEFAULT, attributes); + t->InstanceTemplate()->SetAccessor(env->bytes_read_string(), + GetBytesRead, + nullptr, + env->as_external(), + v8::DEFAULT, + attributes); + env->SetProtoMethod(t, "readStart", JSMethod); env->SetProtoMethod(t, "readStop", JSMethod); if ((flags & kFlagNoShutdown) == 0) @@ -79,6 +86,15 @@ void StreamBase::GetFD(Local key, } +template +void StreamBase::GetBytesRead(Local key, + const PropertyCallbackInfo& args) { + StreamBase* wrap = Unwrap(args.Holder()); + + args.GetReturnValue().Set(wrap->bytes_read_); +} + + template void StreamBase::GetExternal(Local key, const PropertyCallbackInfo& args) { diff --git a/src/stream_base.h b/src/stream_base.h index fad2ddd2e086f0..8c50930abffb01 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -136,7 +136,7 @@ class StreamResource { uv_handle_type pending, void* ctx); - StreamResource() { + StreamResource() : bytes_read_(0) { } virtual ~StreamResource() = default; @@ -160,9 +160,11 @@ class StreamResource { alloc_cb_.fn(size, buf, alloc_cb_.ctx); } - inline void OnRead(size_t nread, + inline void OnRead(ssize_t nread, const uv_buf_t* buf, uv_handle_type pending = UV_UNKNOWN_HANDLE) { + if (nread > 0) + bytes_read_ += nread; if (!read_cb_.is_empty()) read_cb_.fn(nread, buf, pending, read_cb_.ctx); } @@ -182,6 +184,9 @@ class StreamResource { Callback after_write_cb_; Callback alloc_cb_; Callback read_cb_; + int bytes_read_; + + friend class StreamBase; }; class StreamBase : public StreamResource { @@ -249,6 +254,10 @@ class StreamBase : public StreamResource { static void GetExternal(v8::Local key, const v8::PropertyCallbackInfo& args); + template + static void GetBytesRead(v8::Local key, + const v8::PropertyCallbackInfo& args); + template & args)> diff --git a/test/parallel/test-net-bytes-read.js b/test/parallel/test-net-bytes-read.js new file mode 100644 index 00000000000000..32232b29f8d249 --- /dev/null +++ b/test/parallel/test-net-bytes-read.js @@ -0,0 +1,38 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); + +const big = new Buffer(1024 * 1024); +big.fill('-'); + +const server = net.createServer((socket) => { + socket.end(big); + server.close(); +}).listen(common.PORT, () => { + let prev = 0; + + function checkRaise(value) { + assert(value > prev); + prev = value; + } + + const socket = net.connect(common.PORT, () => { + socket.on('data', (chunk) => { + checkRaise(socket.bytesRead); + }); + + socket.on('end', common.mustCall(() => { + assert.equal(socket.bytesRead, prev); + assert.equal(big.length, prev); + })); + + socket.on('close', common.mustCall(() => { + assert(!socket._handle); + assert.equal(socket.bytesRead, prev); + assert.equal(big.length, prev); + })); + }); + socket.end(); +}); From a6ee571252d36ca869b63c333d25dc09e04d722e Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Wed, 20 Apr 2016 11:38:08 -0400 Subject: [PATCH 2/6] fixes --- src/stream_base-inl.h | 3 ++- src/stream_base.h | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index 522ba0ed7d3ba5..9b5ba50bd81e68 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -91,7 +91,8 @@ void StreamBase::GetBytesRead(Local key, const PropertyCallbackInfo& args) { StreamBase* wrap = Unwrap(args.Holder()); - args.GetReturnValue().Set(wrap->bytes_read_); + // int64_t -> double. 53bits is enough for all real cases. + args.GetReturnValue().Set(static_cast(wrap->bytes_read_)); } diff --git a/src/stream_base.h b/src/stream_base.h index 8c50930abffb01..e722a208a8af68 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -164,7 +164,7 @@ class StreamResource { const uv_buf_t* buf, uv_handle_type pending = UV_UNKNOWN_HANDLE) { if (nread > 0) - bytes_read_ += nread; + bytes_read_ += static_cast(nread); if (!read_cb_.is_empty()) read_cb_.fn(nread, buf, pending, read_cb_.ctx); } @@ -184,7 +184,7 @@ class StreamResource { Callback after_write_cb_; Callback alloc_cb_; Callback read_cb_; - int bytes_read_; + uint64_t bytes_read_; friend class StreamBase; }; From 6357f469a89ab1f6fbf7560aebff545b66785d69 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Wed, 20 Apr 2016 11:41:27 -0400 Subject: [PATCH 3/6] symbol --- lib/net.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/net.js b/lib/net.js index 805e878bf45451..2ce2efe3b283d5 100644 --- a/lib/net.js +++ b/lib/net.js @@ -111,6 +111,10 @@ function initSocketHandle(self) { } } + +const BYTES_READ = Symbol('bytesRead'); + + function Socket(options) { if (!(this instanceof Socket)) return new Socket(options); @@ -180,7 +184,7 @@ function Socket(options) { this._server = null; // Used after `.destroy()` - this._bytesRead = 0; + this[BYTES_READ] = 0; } util.inherits(Socket, stream.Duplex); @@ -473,7 +477,7 @@ Socket.prototype._destroy = function(exception, cb) { debug('close handle'); var isException = exception ? true : false; // `bytesRead` should be accessible after `.destroy()` - this._bytesRead = this._handle.bytesRead; + this[BYTES_READ] = this._handle.bytesRead; this._handle.close(() => { debug('emit close'); @@ -582,7 +586,7 @@ Socket.prototype._getpeername = function() { }; Socket.prototype.__defineGetter__('bytesRead', function() { - return this._handle ? this._handle.bytesRead : this._bytesRead; + return this._handle ? this._handle.bytesRead : this[BYTES_READ]; }); Socket.prototype.__defineGetter__('remoteAddress', function() { From 783cae24aa72e6c3cf7a3b2ccaf9b5537be4bd4b Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Wed, 20 Apr 2016 11:55:19 -0400 Subject: [PATCH 4/6] fix test --- test/parallel/test-net-bytes-read.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/parallel/test-net-bytes-read.js b/test/parallel/test-net-bytes-read.js index 32232b29f8d249..ba2bc160d0a982 100644 --- a/test/parallel/test-net-bytes-read.js +++ b/test/parallel/test-net-bytes-read.js @@ -4,8 +4,7 @@ const common = require('../common'); const assert = require('assert'); const net = require('net'); -const big = new Buffer(1024 * 1024); -big.fill('-'); +const big = Buffer.alloc(1024 * 1024); const server = net.createServer((socket) => { socket.end(big); From 4c50cc4ff13342906b79445f53f01229decb3d8b Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Wed, 20 Apr 2016 11:59:16 -0400 Subject: [PATCH 5/6] net: replace __defineGetter__ with defineProperty `Object.prototype.__defineGetter__` is deprecated now, use `Object.defineProperty` instead. --- lib/net.js | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/net.js b/lib/net.js index 2ce2efe3b283d5..4730a87de2b696 100644 --- a/lib/net.js +++ b/lib/net.js @@ -585,19 +585,27 @@ Socket.prototype._getpeername = function() { return this._peername; }; -Socket.prototype.__defineGetter__('bytesRead', function() { +function protoGetter(name, callback) { + Object.defineProperty(Socket.prototype, name, { + configurable: false, + enumerable: true, + get: callback + }); +} + +protoGetter('bytesRead', function bytesRead() { return this._handle ? this._handle.bytesRead : this[BYTES_READ]; }); -Socket.prototype.__defineGetter__('remoteAddress', function() { +protoGetter('remoteAddress', function remoteAddress() { return this._getpeername().address; }); -Socket.prototype.__defineGetter__('remoteFamily', function() { +protoGetter('remoteFamily', function remoteFamily() { return this._getpeername().family; }); -Socket.prototype.__defineGetter__('remotePort', function() { +protoGetter('remotePort', function remotePort() { return this._getpeername().port; }); @@ -616,12 +624,12 @@ Socket.prototype._getsockname = function() { }; -Socket.prototype.__defineGetter__('localAddress', function() { +protoGetter('localAddress', function localAddress() { return this._getsockname().address; }); -Socket.prototype.__defineGetter__('localPort', function() { +protoGetter('localPort', function localPort() { return this._getsockname().port; }); @@ -735,7 +743,7 @@ function createWriteReq(req, handle, data, encoding) { } -Socket.prototype.__defineGetter__('bytesWritten', function() { +protoGetter('bytesWritten', function bytesWritten() { var bytes = this._bytesDispatched; const state = this._writableState; const data = this._pendingData; From 62a88cb223a54583cb2ba7a411ee80f98807af79 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Wed, 20 Apr 2016 12:37:04 -0400 Subject: [PATCH 6/6] fix --- src/stream_base-inl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index 9b5ba50bd81e68..099e105334cedf 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -91,7 +91,7 @@ void StreamBase::GetBytesRead(Local key, const PropertyCallbackInfo& args) { StreamBase* wrap = Unwrap(args.Holder()); - // int64_t -> double. 53bits is enough for all real cases. + // uint64_t -> double. 53bits is enough for all real cases. args.GetReturnValue().Set(static_cast(wrap->bytes_read_)); }