141268 2015-02-04 15:24:35 -0800 r179576 introduce a deadlock potential during GC thread suspension 2015-02-04 19:52:38 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 mark.lam mark.lam fpizlo ggaren gyuyoung.kim mmirman msaboff oliver oldest_to_newest 1066961 0 mark.lam 2015-02-04 15:24:35 -0800 http://trac.webkit.org/r179576 introduced a potential for deadlocking. In the GC thread suspension loop, we currently delete MachineThreads::Thread that we detect to be invalid. This is unsafe because we may have already suspended some threads, and one of those suspended threads may still be holding the heap lock. An attempt to do a deletion there may therefore result in a deadlock. The fix is to put to the invalid threads in a separate toBeDeleted list, and delete them only after GC has resumed all threads. 1066962 1 mark.lam 2015-02-04 15:26:08 -0800 This is also being tracked in <rdar://problem/17979784> as a follow up fix. 1066965 2 246061 mark.lam 2015-02-04 15:35:48 -0800 Created attachment 246061 the patch. 1066978 3 246061 msaboff 2015-02-04 16:14:32 -0800 Comment on attachment 246061 the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=246061&action=review r=me > Source/JavaScriptCore/ChangeLog:15 > + The fix is to put to the invalid threads in a separate toBeDeleted list, Wording. Did you mean "The fix is to put the invalid threads ..."? > Source/JavaScriptCore/heap/MachineStackMarker.cpp:479 > + // Put the invalid thread on the threadsToBeDeleted list. > + // We can't just delete it here because we have suspended other > + // threads, and they may still be holding the heap lock. An > + // attempt to delete the invalid thread here may therefore > + // result in a deadlock. Hence, we need to defer the deletion > + // till after we have resumed all threads. Simplify the comment as we discussed in person. 1066983 4 mark.lam 2015-02-04 16:34:01 -0800 Thanks for the review. I've simplified the comments and fixed the typo. Landed in r179648: <http://trac.webkit.org/r179648>. 1067024 5 gyuyoung.kim 2015-02-04 19:52:38 -0800 (In reply to comment #4) > Thanks for the review. I've simplified the comments and fixed the typo. > > Landed in r179648: <http://trac.webkit.org/r179648>. I fix a build break on EFL port since r179648. - http://trac.webkit.org/changeset/179666 246061 2015-02-04 15:35:48 -0800 2015-02-04 16:14:32 -0800 the patch. bug-141268.patch text/plain 6651 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTc5NjQwKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDMyIEBA CisyMDE1LTAyLTA0ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBy MTc5NTc2IGludHJvZHVjZSBhIGRlYWRsb2NrIHBvdGVudGlhbCBkdXJpbmcgR0MgdGhyZWFkIHN1 c3BlbnNpb24uCisgICAgICAgIDxodHRwczovL3dlYmtpdC5vcmcvYi8xNDEyNjg+CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgaHR0cDovL3RyYWMud2Vi a2l0Lm9yZy9yMTc5NTc2IGludHJvZHVjZWQgYSBwb3RlbnRpYWwgZm9yIGRlYWRsb2NraW5nLgor ICAgICAgICBJbiB0aGUgR0MgdGhyZWFkIHN1c3BlbnNpb24gbG9vcCwgd2UgY3VycmVudGx5IGRl bGV0ZQorICAgICAgICBNYWNoaW5lVGhyZWFkczo6VGhyZWFkIHRoYXQgd2UgZGV0ZWN0IHRvIGJl IGludmFsaWQuICBUaGlzIGlzIHVuc2FmZQorICAgICAgICBiZWNhdXNlIHdlIG1heSBoYXZlIGFs cmVhZHkgc3VzcGVuZGVkIHNvbWUgdGhyZWFkcywgYW5kIG9uZSBvZiB0aG9zZQorICAgICAgICBz dXNwZW5kZWQgdGhyZWFkcyBtYXkgc3RpbGwgYmUgaG9sZGluZyB0aGUgaGVhcCBsb2NrLiAgQW4g YXR0ZW1wdCB0byBkbworICAgICAgICBhIGRlbGV0aW9uIHRoZXJlIG1heSB0aGVyZWZvcmUgcmVz dWx0IGluIGEgZGVhZGxvY2suCisKKyAgICAgICAgVGhlIGZpeCBpcyB0byBwdXQgdG8gdGhlIGlu dmFsaWQgdGhyZWFkcyBpbiBhIHNlcGFyYXRlIHRvQmVEZWxldGVkIGxpc3QsCisgICAgICAgIGFu ZCBkZWxldGUgdGhlbSBvbmx5IGFmdGVyIEdDIGhhcyByZXN1bWVkIGFsbCB0aHJlYWRzLgorCisg ICAgICAgICogaGVhcC9NYWNoaW5lU3RhY2tNYXJrZXIuY3BwOgorICAgICAgICAoSlNDOjpNYWNo aW5lVGhyZWFkczo6cmVtb3ZlQ3VycmVudFRocmVhZCk6CisgICAgICAgIC0gVW5kbyByZWZhY3Rv cmluZyByZW1vdmVUaHJlYWRXaXRoTG9ja0FscmVhZHlBY3F1aXJlZCgpIG91dCBvZgorICAgICAg ICAgIHJlbW92ZUN1cnJlbnRUaHJlYWQoKSBzaW5jZSBpdCBpcyBubyBsb25nZXIgbmVlZGVkLgor CisgICAgICAgIChKU0M6Ok1hY2hpbmVUaHJlYWRzOjp0cnlDb3B5T3RoZXJUaHJlYWRTdGFja3Mp OgorICAgICAgICAtIFB1dCBpbnZhbGlkIFRocmVhZHMgb24gYSB0aHJlYWRzVG9CZURlbGV0ZWQg bGlzdCwgYW5kIGRlbGV0ZSB0aG9zZQorICAgICAgICAgIFRocmVhZHMgb25seSBhZnRlciBhbGwg dGhyZWFkcyBoYXZlIGJlZW4gcmVzdW1lZC4KKworICAgICAgICAoSlNDOjpNYWNoaW5lVGhyZWFk czo6cmVtb3ZlVGhyZWFkV2l0aExvY2tBbHJlYWR5QWNxdWlyZWQpOiBEZWxldGVkLgorICAgICAg ICAqIGhlYXAvTWFjaGluZVN0YWNrTWFya2VyLmg6CisKIDIwMTUtMDItMDQgIEpvc2VwaCBQZWNv cmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KIAogICAgICAgICBXZWIgSW5zcGVjdG9yOiBDbGVh biB1cCBPYmplY3QgUHJvcGVydHkgRGVzY3JpcHRvciBDb2xsZWN0aW9uCkluZGV4OiBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvaGVhcC9NYWNoaW5lU3RhY2tNYXJrZXIuY3BwCj09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0t IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9oZWFwL01hY2hpbmVTdGFja01hcmtlci5jcHAJKHJldmlz aW9uIDE3OTYzOCkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9oZWFwL01hY2hpbmVTdGFja01h cmtlci5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTE4MywxMCArMTgzLDEyIEBAIHZvaWQgTWFjaGlu ZVRocmVhZHM6OnJlbW92ZVRocmVhZCh2b2lkKiAKICAgICBzdGF0aWNfY2FzdDxNYWNoaW5lVGhy ZWFkcyo+KHApLT5yZW1vdmVDdXJyZW50VGhyZWFkKCk7CiB9CiAKLXRlbXBsYXRlPHR5cGVuYW1l IFBsYXRmb3JtVGhyZWFkPgotdm9pZCBNYWNoaW5lVGhyZWFkczo6cmVtb3ZlVGhyZWFkV2l0aExv Y2tBbHJlYWR5QWNxdWlyZWQoUGxhdGZvcm1UaHJlYWQgdGhyZWFkVG9SZW1vdmUpCit2b2lkIE1h Y2hpbmVUaHJlYWRzOjpyZW1vdmVDdXJyZW50VGhyZWFkKCkKIHsKLSAgICBpZiAoZXF1YWxUaHJl YWQodGhyZWFkVG9SZW1vdmUsIG1fcmVnaXN0ZXJlZFRocmVhZHMtPnBsYXRmb3JtVGhyZWFkKSkg eworICAgIFBsYXRmb3JtVGhyZWFkIGN1cnJlbnRQbGF0Zm9ybVRocmVhZCA9IGdldEN1cnJlbnRQ bGF0Zm9ybVRocmVhZCgpOworCisgICAgTXV0ZXhMb2NrZXIgbG9jayhtX3JlZ2lzdGVyZWRUaHJl YWRzTXV0ZXgpOworICAgIGlmIChlcXVhbFRocmVhZChjdXJyZW50UGxhdGZvcm1UaHJlYWQsIG1f cmVnaXN0ZXJlZFRocmVhZHMtPnBsYXRmb3JtVGhyZWFkKSkgewogICAgICAgICBUaHJlYWQqIHQg PSBtX3JlZ2lzdGVyZWRUaHJlYWRzOwogICAgICAgICBtX3JlZ2lzdGVyZWRUaHJlYWRzID0gbV9y ZWdpc3RlcmVkVGhyZWFkcy0+bmV4dDsKICAgICAgICAgZGVsZXRlIHQ7CkBAIC0xOTQsNyArMTk2 LDcgQEAgdm9pZCBNYWNoaW5lVGhyZWFkczo6cmVtb3ZlVGhyZWFkV2l0aExvYwogICAgICAgICBU aHJlYWQqIGxhc3QgPSBtX3JlZ2lzdGVyZWRUaHJlYWRzOwogICAgICAgICBUaHJlYWQqIHQ7CiAg ICAgICAgIGZvciAodCA9IG1fcmVnaXN0ZXJlZFRocmVhZHMtPm5leHQ7IHQ7IHQgPSB0LT5uZXh0 KSB7Ci0gICAgICAgICAgICBpZiAoZXF1YWxUaHJlYWQodC0+cGxhdGZvcm1UaHJlYWQsIHRocmVh ZFRvUmVtb3ZlKSkgeworICAgICAgICAgICAgaWYgKGVxdWFsVGhyZWFkKHQtPnBsYXRmb3JtVGhy ZWFkLCBjdXJyZW50UGxhdGZvcm1UaHJlYWQpKSB7CiAgICAgICAgICAgICAgICAgbGFzdC0+bmV4 dCA9IHQtPm5leHQ7CiAgICAgICAgICAgICAgICAgYnJlYWs7CiAgICAgICAgICAgICB9CkBAIC0y MDUsMTQgKzIwNyw2IEBAIHZvaWQgTWFjaGluZVRocmVhZHM6OnJlbW92ZVRocmVhZFdpdGhMb2MK ICAgICB9CiB9CiAgICAgCi12b2lkIE1hY2hpbmVUaHJlYWRzOjpyZW1vdmVDdXJyZW50VGhyZWFk KCkKLXsKLSAgICBQbGF0Zm9ybVRocmVhZCBjdXJyZW50UGxhdGZvcm1UaHJlYWQgPSBnZXRDdXJy ZW50UGxhdGZvcm1UaHJlYWQoKTsKLQotICAgIE11dGV4TG9ja2VyIGxvY2sobV9yZWdpc3RlcmVk VGhyZWFkc011dGV4KTsKLSAgICByZW1vdmVUaHJlYWRXaXRoTG9ja0FscmVhZHlBY3F1aXJlZChj dXJyZW50UGxhdGZvcm1UaHJlYWQpOwotfQotICAgIAogdm9pZCBNYWNoaW5lVGhyZWFkczo6Z2F0 aGVyRnJvbUN1cnJlbnRUaHJlYWQoQ29uc2VydmF0aXZlUm9vdHMmIGNvbnNlcnZhdGl2ZVJvb3Rz LCBKSVRTdHViUm91dGluZVNldCYgaml0U3R1YlJvdXRpbmVzLCBDb2RlQmxvY2tTZXQmIGNvZGVC bG9ja3MsIHZvaWQqIHN0YWNrQ3VycmVudCwgUmVnaXN0ZXJTdGF0ZSYgcmVnaXN0ZXJzKQogewog ICAgIHZvaWQqIHJlZ2lzdGVyc0JlZ2luID0gJnJlZ2lzdGVyczsKQEAgLTQ1Niw3ICs0NTAsOSBA QCBib29sIE1hY2hpbmVUaHJlYWRzOjp0cnlDb3B5T3RoZXJUaHJlYWRTCiAgICAgUGxhdGZvcm1U aHJlYWQgY3VycmVudFBsYXRmb3JtVGhyZWFkID0gZ2V0Q3VycmVudFBsYXRmb3JtVGhyZWFkKCk7 CiAgICAgaW50IG51bWJlck9mVGhyZWFkcyA9IDA7IC8vIFVzaW5nIDAgdG8gZGVub3RlIHRoYXQg d2UgaGF2ZW4ndCBjb3VudGVkIHRoZSBudW1iZXIgb2YgdGhyZWFkcyB5ZXQuCiAgICAgaW50IGlu ZGV4ID0gMTsKKyAgICBUaHJlYWQqIHRocmVhZHNUb0JlRGVsZXRlZCA9IG51bGxwdHI7CiAKKyAg ICBUaHJlYWQqIHByZXZpb3VzVGhyZWFkID0gbnVsbHB0cjsKICAgICBmb3IgKFRocmVhZCogdGhy ZWFkID0gbV9yZWdpc3RlcmVkVGhyZWFkczsgdGhyZWFkOyBpbmRleCsrKSB7CiAgICAgICAgIGlm ICghZXF1YWxUaHJlYWQodGhyZWFkLT5wbGF0Zm9ybVRocmVhZCwgY3VycmVudFBsYXRmb3JtVGhy ZWFkKSkgewogICAgICAgICAgICAgYm9vbCBzdWNjZXNzID0gc3VzcGVuZFRocmVhZCh0aHJlYWQt PnBsYXRmb3JtVGhyZWFkKTsKQEAgLTQ3NCw5ICs0NzAsMjEgQEAgYm9vbCBNYWNoaW5lVGhyZWFk czo6dHJ5Q29weU90aGVyVGhyZWFkUwogICAgICAgICAgICAgICAgIFdURlJlcG9ydEVycm9yKF9f RklMRV9fLCBfX0xJTkVfXywgV1RGX1BSRVRUWV9GVU5DVElPTiwKICAgICAgICAgICAgICAgICAg ICAgIkphdmFTY3JpcHQgZ2FyYmFnZSBjb2xsZWN0aW9uIGVuY291bnRlcmVkIGFuIGludmFsaWQg dGhyZWFkIChlcnIgMHgleCk6IFRocmVhZCBbJWQvJWQ6ICVwXSBwbGF0Zm9ybVRocmVhZCAlcC4i LAogICAgICAgICAgICAgICAgICAgICBlcnJvciwgaW5kZXgsIG51bWJlck9mVGhyZWFkcywgdGhy ZWFkLCByZWludGVycHJldF9jYXN0PHZvaWQqPih0aHJlYWQtPnBsYXRmb3JtVGhyZWFkKSk7Ci0g ICAgICAgICAgICAgICAgCisKKyAgICAgICAgICAgICAgICAvLyBQdXQgdGhlIGludmFsaWQgdGhy ZWFkIG9uIHRoZSB0aHJlYWRzVG9CZURlbGV0ZWQgbGlzdC4KKyAgICAgICAgICAgICAgICAvLyBX ZSBjYW4ndCBqdXN0IGRlbGV0ZSBpdCBoZXJlIGJlY2F1c2Ugd2UgaGF2ZSBzdXNwZW5kZWQgb3Ro ZXIKKyAgICAgICAgICAgICAgICAvLyB0aHJlYWRzLCBhbmQgdGhleSBtYXkgc3RpbGwgYmUgaG9s ZGluZyB0aGUgaGVhcCBsb2NrLiBBbgorICAgICAgICAgICAgICAgIC8vIGF0dGVtcHQgdG8gZGVs ZXRlIHRoZSBpbnZhbGlkIHRocmVhZCBoZXJlIG1heSB0aGVyZWZvcmUKKyAgICAgICAgICAgICAg ICAvLyByZXN1bHQgaW4gYSBkZWFkbG9jay4gSGVuY2UsIHdlIG5lZWQgdG8gZGVmZXIgdGhlIGRl bGV0aW9uCisgICAgICAgICAgICAgICAgLy8gdGlsbCBhZnRlciB3ZSBoYXZlIHJlc3VtZWQgYWxs IHRocmVhZHMuCiAgICAgICAgICAgICAgICAgVGhyZWFkKiBuZXh0VGhyZWFkID0gdGhyZWFkLT5u ZXh0OwotICAgICAgICAgICAgICAgIHJlbW92ZVRocmVhZFdpdGhMb2NrQWxyZWFkeUFjcXVpcmVk KHRocmVhZC0+cGxhdGZvcm1UaHJlYWQpOworICAgICAgICAgICAgICAgIHRocmVhZC0+bmV4dCA9 IHRocmVhZHNUb0JlRGVsZXRlZDsKKyAgICAgICAgICAgICAgICB0aHJlYWRzVG9CZURlbGV0ZWQg PSB0aHJlYWQ7CisKKyAgICAgICAgICAgICAgICBpZiAocHJldmlvdXNUaHJlYWQpCisgICAgICAg ICAgICAgICAgICAgIHByZXZpb3VzVGhyZWFkLT5uZXh0ID0gbmV4dFRocmVhZDsKKyAgICAgICAg ICAgICAgICBlbHNlCisgICAgICAgICAgICAgICAgICAgIG1fcmVnaXN0ZXJlZFRocmVhZHMgPSBu ZXh0VGhyZWFkOwogICAgICAgICAgICAgICAgIHRocmVhZCA9IG5leHRUaHJlYWQ7CiAgICAgICAg ICAgICAgICAgY29udGludWU7CiAgICAgICAgICAgICB9CkBAIC00ODUsNiArNDkzLDcgQEAgYm9v bCBNYWNoaW5lVGhyZWFkczo6dHJ5Q29weU90aGVyVGhyZWFkUwogICAgICAgICAgICAgQVNTRVJU X1VOVVNFRChzdWNjZXNzLCBzdWNjZXNzKTsKICNlbmRpZgogICAgICAgICB9CisgICAgICAgIHBy ZXZpb3VzVGhyZWFkID0gdGhyZWFkOwogICAgICAgICB0aHJlYWQgPSB0aHJlYWQtPm5leHQ7CiAg ICAgfQogCkBAIC00OTgsNiArNTA3LDEyIEBAIGJvb2wgTWFjaGluZVRocmVhZHM6OnRyeUNvcHlP dGhlclRocmVhZFMKICAgICAgICAgICAgIHJlc3VtZVRocmVhZCh0aHJlYWQtPnBsYXRmb3JtVGhy ZWFkKTsKICAgICB9CiAKKyAgICBmb3IgKFRocmVhZCogdGhyZWFkID0gdGhyZWFkc1RvQmVEZWxl dGVkOyB0aHJlYWQ7ICkgeworICAgICAgICBUaHJlYWQqIG5leHRUaHJlYWQgPSB0aHJlYWQtPm5l eHQ7CisgICAgICAgIGRlbGV0ZSB0aHJlYWQ7CisgICAgICAgIHRocmVhZCA9IG5leHRUaHJlYWQ7 CisgICAgfQorICAgIAogICAgIHJldHVybiAqc2l6ZSA8PSBjYXBhY2l0eTsKIH0KIApJbmRleDog U291cmNlL0phdmFTY3JpcHRDb3JlL2hlYXAvTWFjaGluZVN0YWNrTWFya2VyLmgKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2hlYXAvTWFjaGluZVN0YWNrTWFya2VyLmgJKHJl dmlzaW9uIDE3OTYzOCkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9oZWFwL01hY2hpbmVTdGFj a01hcmtlci5oCSh3b3JraW5nIGNvcHkpCkBAIC01Nyw5ICs1Nyw2IEBAIG5hbWVzcGFjZSBKU0Mg ewogICAgICAgICBzdGF0aWMgdm9pZCByZW1vdmVUaHJlYWQodm9pZCopOwogICAgICAgICB2b2lk IHJlbW92ZUN1cnJlbnRUaHJlYWQoKTsKIAotICAgICAgICB0ZW1wbGF0ZTx0eXBlbmFtZSBQbGF0 Zm9ybVRocmVhZD4KLSAgICAgICAgdm9pZCByZW1vdmVUaHJlYWRXaXRoTG9ja0FscmVhZHlBY3F1 aXJlZChQbGF0Zm9ybVRocmVhZCk7Ci0KICAgICAgICAgTXV0ZXggbV9yZWdpc3RlcmVkVGhyZWFk c011dGV4OwogICAgICAgICBUaHJlYWQqIG1fcmVnaXN0ZXJlZFRocmVhZHM7CiAgICAgICAgIFdU Rjo6VGhyZWFkU3BlY2lmaWNLZXkgbV90aHJlYWRTcGVjaWZpYzsK