-
Notifications
You must be signed in to change notification settings - Fork 29.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Segfault in TLSWrap in v5.5.0 and master. #5108
Comments
cc @nodejs/crypto |
mscdex
added
the
c++
Issues and PRs that require attention from people who are familiar with C++.
label
Feb 5, 2016
ChALkeR
changed the title
Segfault in TLSWrap in v5.5.0
Segfault in TLSWrap in v5.5.0 and master.
Feb 5, 2016
Ok, I merged all my comments into the original post. |
Full debug stack trace:
|
@ChALkeR may I ask you to give a try to this fix? diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
index a888dc1..c12b2a8 100644
--- a/lib/_tls_wrap.js
+++ b/lib/_tls_wrap.js
@@ -299,6 +299,8 @@ proxiedMethods.forEach(function(name) {
});
tls_wrap.TLSWrap.prototype.close = function closeProxy(cb) {
+ if (this.owner)
+ this.owner.ssl = null;
if (this._parentWrap && this._parentWrap._handle === this._parent) {
this._parentWrap.once('close', cb);
return this._parentWrap.destroy(); |
@indutny Will test that soon, sorry. |
indutny
added a commit
to indutny/io.js
that referenced
this issue
Feb 9, 2016
This is an intermediate fix for an issue of accessing `TLSWrap` fields after the parent handle was destroyed. While `close` listener cleans up this field automatically, it can be done even earlier at the `TLSWrap.close` call. Proper fix is going to be submitted and landed after this one. Fix: nodejs#5108
@indutny The fix works for me in both the small (published above) and the original testcase, thanks! |
rvagg
pushed a commit
that referenced
this issue
Feb 18, 2016
This is an intermediate fix for an issue of accessing `TLSWrap` fields after the parent handle was destroyed. While `close` listener cleans up this field automatically, it can be done even earlier at the `TLSWrap.close` call. Proper fix is going to be submitted and landed after this one. Fix: #5108 PR-URL: #5168 Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
stefanmb
pushed a commit
to stefanmb/node
that referenced
this issue
Feb 23, 2016
This is an intermediate fix for an issue of accessing `TLSWrap` fields after the parent handle was destroyed. While `close` listener cleans up this field automatically, it can be done even earlier at the `TLSWrap.close` call. Proper fix is going to be submitted and landed after this one. Fix: nodejs#5108 PR-URL: nodejs#5168 Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
MylesBorins
pushed a commit
that referenced
this issue
Mar 1, 2016
This is an intermediate fix for an issue of accessing `TLSWrap` fields after the parent handle was destroyed. While `close` listener cleans up this field automatically, it can be done even earlier at the `TLSWrap.close` call. Proper fix is going to be submitted and landed after this one. Fix: #5108 PR-URL: #5168 Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
MylesBorins
pushed a commit
that referenced
this issue
Mar 1, 2016
This is an intermediate fix for an issue of accessing `TLSWrap` fields after the parent handle was destroyed. While `close` listener cleans up this field automatically, it can be done even earlier at the `TLSWrap.close` call. Proper fix is going to be submitted and landed after this one. Fix: #5108 PR-URL: #5168 Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
MylesBorins
pushed a commit
that referenced
this issue
Mar 2, 2016
This is an intermediate fix for an issue of accessing `TLSWrap` fields after the parent handle was destroyed. While `close` listener cleans up this field automatically, it can be done even earlier at the `TLSWrap.close` call. Proper fix is going to be submitted and landed after this one. Fix: #5108 PR-URL: #5168 Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
ryzokuken
added a commit
to ryzokuken/node
that referenced
this issue
Mar 18, 2018
Rename the tests appropriately alongside mentioning the subsystem Also, make a few basic changes to make sure the tests conform to the standard test structure 1. Renamed test-regress-GH-io-1068 to test-tty-stdin-end 2. Renamed test-regress-GH-io-1811 to test-zlib-kmaxlength-rangeerror 3. Renamed test-regress-GH-node-9326 to test-kill-segfault-freebsd 4. Renamed test-timers-regress-nodejsGH-9765 to test-timers-setimmediate-infinite-loop 5. Renamed test-tls-pfx-nodejsgh-5100-regr to test-tls-pfx-authorizationerror 6. Renamed test-tls-regr-nodejsgh-5108 to test-tls-tlswrap-segfault Fixes: nodejs#19105 Refs: nodejs#19105 Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure
lpinca
pushed a commit
that referenced
this issue
Mar 18, 2018
Rename the tests appropriately alongside mentioning the subsystem. Also, make a few basic changes to make sure the tests conform to the standard test structure. - Rename test-regress-GH-io-1068 to test-tty-stdin-end - Rename test-regress-GH-io-1811 to test-zlib-kmaxlength-rangeerror - Rename test-regress-GH-node-9326 to test-kill-segfault-freebsd - Rename test-timers-regress-GH-9765 to test-timers-setimmediate-infinite-loop - Rename test-tls-pfx-gh-5100-regr to test-tls-pfx-authorizationerror - Rename test-tls-regr-gh-5108 to test-tls-tlswrap-segfault PR-URL: #19332 Fixes: #19105 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Weijia Wang <starkwang@126.com> Reviewed-By: Yuta Hiroto <hello@hiroppy.me> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
MylesBorins
pushed a commit
that referenced
this issue
Mar 20, 2018
Rename the tests appropriately alongside mentioning the subsystem. Also, make a few basic changes to make sure the tests conform to the standard test structure. - Rename test-regress-GH-io-1068 to test-tty-stdin-end - Rename test-regress-GH-io-1811 to test-zlib-kmaxlength-rangeerror - Rename test-regress-GH-node-9326 to test-kill-segfault-freebsd - Rename test-timers-regress-GH-9765 to test-timers-setimmediate-infinite-loop - Rename test-tls-pfx-gh-5100-regr to test-tls-pfx-authorizationerror - Rename test-tls-regr-gh-5108 to test-tls-tlswrap-segfault PR-URL: #19332 Fixes: #19105 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Weijia Wang <starkwang@126.com> Reviewed-By: Yuta Hiroto <hello@hiroppy.me> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
MylesBorins
pushed a commit
that referenced
this issue
Mar 20, 2018
Rename the tests appropriately alongside mentioning the subsystem. Also, make a few basic changes to make sure the tests conform to the standard test structure. - Rename test-regress-GH-io-1068 to test-tty-stdin-end - Rename test-regress-GH-io-1811 to test-zlib-kmaxlength-rangeerror - Rename test-regress-GH-node-9326 to test-kill-segfault-freebsd - Rename test-timers-regress-GH-9765 to test-timers-setimmediate-infinite-loop - Rename test-tls-pfx-gh-5100-regr to test-tls-pfx-authorizationerror - Rename test-tls-regr-gh-5108 to test-tls-tlswrap-segfault PR-URL: #19332 Fixes: #19105 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Weijia Wang <starkwang@126.com> Reviewed-By: Yuta Hiroto <hello@hiroppy.me> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
BethGriggs
pushed a commit
that referenced
this issue
Dec 3, 2018
Rename the tests appropriately alongside mentioning the subsystem. Also, make a few basic changes to make sure the tests conform to the standard test structure. - Rename test-regress-GH-io-1068 to test-tty-stdin-end - Rename test-regress-GH-io-1811 to test-zlib-kmaxlength-rangeerror - Rename test-regress-GH-node-9326 to test-kill-segfault-freebsd - Rename test-timers-regress-GH-9765 to test-timers-setimmediate-infinite-loop - Rename test-tls-pfx-gh-5100-regr to test-tls-pfx-authorizationerror - Rename test-tls-regr-gh-5108 to test-tls-tlswrap-segfault PR-URL: #19332 Fixes: #19105 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Weijia Wang <starkwang@126.com> Reviewed-By: Yuta Hiroto <hello@hiroppy.me> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Edit: updated, line numbers are against current master (1124de2) now.
Current master, v4.2.6, v5.5.0 are affected. Bisect points at 75930bb.
It crashes at tls_wrap.cc#L521.
/cc @indutny
Testcase (looks like a race to me, not guaranteed to reproduce everywhere):
The text was updated successfully, but these errors were encountered: