Skip to content

Commit

Permalink
tls_wrap: do not hold persistent ref to parent
Browse files Browse the repository at this point in the history
Hold non-persistent reference in JS, rather than in C++ to avoid cycles.

PR-URL: #1078
Reviewed-By: Ben Noordhuis <[email protected]>
  • Loading branch information
indutny committed Mar 6, 2015
1 parent dccb69a commit c09c90c
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 15 deletions.
1 change: 1 addition & 0 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ TLSSocket.prototype._wrapHandle = function(handle) {
tls.createSecureContext();
res = tls_wrap.wrap(handle, context.context, options.isServer);
res._parent = handle;
res._secureContext = context;
res.reading = handle.reading;
Object.defineProperty(handle, 'reading', {
get: function readingGetter() {
Expand Down
16 changes: 5 additions & 11 deletions src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,14 @@ using v8::Value;
TLSWrap::TLSWrap(Environment* env,
Kind kind,
StreamBase* stream,
Handle<Object> stream_obj,
Handle<Object> sc)
: SSLWrap<TLSWrap>(env, Unwrap<SecureContext>(sc), kind),
SecureContext* sc)
: SSLWrap<TLSWrap>(env, sc, kind),
StreamBase(env),
AsyncWrap(env,
env->tls_wrap_constructor_function()->NewInstance(),
AsyncWrap::PROVIDER_TLSWRAP),
sc_(Unwrap<SecureContext>(sc)),
sc_handle_(env->isolate(), sc),
sc_(sc),
stream_(stream),
stream_handle_(env->isolate(), stream_obj),
enc_in_(nullptr),
enc_out_(nullptr),
clear_in_(nullptr),
Expand Down Expand Up @@ -84,9 +81,6 @@ TLSWrap::~TLSWrap() {
clear_in_ = nullptr;

sc_ = nullptr;
sc_handle_.Reset();
stream_handle_.Reset();
persistent().Reset();

#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
sni_context_.Reset();
Expand Down Expand Up @@ -196,9 +190,9 @@ void TLSWrap::Wrap(const FunctionCallbackInfo<Value>& args) {
});
CHECK_NE(stream, nullptr);

TLSWrap* res = new TLSWrap(env, kind, stream, stream_obj, sc);
TLSWrap* res = new TLSWrap(env, kind, stream, Unwrap<SecureContext>(sc));

args.GetReturnValue().Set(res->persistent());
args.GetReturnValue().Set(res->object());
}


Expand Down
5 changes: 1 addition & 4 deletions src/tls_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ class TLSWrap : public crypto::SSLWrap<TLSWrap>,
TLSWrap(Environment* env,
Kind kind,
StreamBase* stream,
v8::Handle<v8::Object> stream_obj,
v8::Handle<v8::Object> sc);
crypto::SecureContext* sc);

static void SSLInfoCallback(const SSL* ssl_, int where, int ret);
void InitSSL();
Expand Down Expand Up @@ -141,9 +140,7 @@ class TLSWrap : public crypto::SSLWrap<TLSWrap>,
#endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB

crypto::SecureContext* sc_;
v8::Persistent<v8::Object> sc_handle_;
StreamBase* stream_;
v8::Persistent<v8::Object> stream_handle_;
BIO* enc_in_;
BIO* enc_out_;
NodeBIO* clear_in_;
Expand Down

0 comments on commit c09c90c

Please sign in to comment.