1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

http2: keep session objects alive during Http2Scope

Browse Source 
Keep a local handle as a reference to the JS `Http2Session`
object so that it will not be garbage collected
when inside an `Http2Scope`, because the presence of the
latter usually indicates that further actions on
the session object are expected.

Strictly speaking, storing the `session_handle_` as a
property on the scope object is not necessary, but
this is not very costly and makes the code more
obviously correct.

Fixes: https://github.com/nodejs/node/issues/17840

PR-URL: https://github.com/nodejs/node/pull/17863
Fixes: https://github.com/nodejs/node/issues/17840
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit is contained in:
Anna Henningsen 2017-12-25 20:16:54 +01:00 committed by James M Snell
parent 57594d2782
commit 06666305da
3 changed files with 19 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/node_http2.cc
View File

@ -71,6 +71,11 @@ Http2Scope::Http2Scope(Http2Session* session) {
} }
session->flags_ |= SESSION_STATE_HAS_SCOPE; session->flags_ |= SESSION_STATE_HAS_SCOPE;
session_ = session; session_ = session;
// Always keep the session object alive for at least as long as
// this scope is active.
session_handle_ = session->object();
CHECK(!session_handle_.IsEmpty());
} }
Http2Scope::~Http2Scope() { Http2Scope::~Http2Scope() {
@ -512,6 +517,7 @@ void Http2Session::Unconsume() {
} }
Http2Session::~Http2Session() { Http2Session::~Http2Session() {
CHECK_EQ(flags_ & SESSION_STATE_HAS_SCOPE, 0);
if (!object().IsEmpty()) if (!object().IsEmpty())
ClearWrap(object()); ClearWrap(object());
persistent().Reset(); persistent().Reset();
@ -1365,6 +1371,7 @@ void Http2Session::OnStreamReadImpl(ssize_t nread,
void* ctx) { void* ctx) {
Http2Session* session = static_cast<Http2Session*>(ctx); Http2Session* session = static_cast<Http2Session*>(ctx);
Http2Scope h2scope(session); Http2Scope h2scope(session);
CHECK_NE(session->stream_, nullptr);
DEBUG_HTTP2SESSION2(session, "receiving %d bytes", nread); DEBUG_HTTP2SESSION2(session, "receiving %d bytes", nread);
if (nread < 0) { if (nread < 0) {
uv_buf_t tmp_buf; uv_buf_t tmp_buf;

1
src/node_http2.h
View File

@ -445,6 +445,7 @@ class Http2Scope {
private: private:
Http2Session* session_ = nullptr; Http2Session* session_ = nullptr;
Local<Object> session_handle_;
}; };
// The Http2Options class is used to parse the options object passed in to // The Http2Options class is used to parse the options object passed in to

11
test/parallel/test-http2-createwritereq.js
View File

@ -1,5 +1,7 @@
'use strict'; 'use strict';
// Flags: --expose-gc
const common = require('../common'); const common = require('../common');
if (!common.hasCrypto) if (!common.hasCrypto)
common.skip('missing crypto'); common.skip('missing crypto');
@ -62,6 +64,15 @@ server.listen(0, common.mustCall(function() {
} }
})); }));
// Ref: https://github.com/nodejs/node/issues/17840
const origDestroy = req.destroy;
req.destroy = function(...args) {
// Schedule a garbage collection event at the end of the current
// MakeCallback() run.
process.nextTick(global.gc);
return origDestroy.call(this, ...args);
};
req.end(); req.end();
}); });
})); }));