132088 2014-04-23 16:26:05 -0700 The GC should only resume compiler threads that it suspended in the same GC pass 2014-04-23 17:44:23 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mark.lam mark.lam fpizlo ggaren mhahnenberg mmirman msaboff oliver webkit-bug-importer oldest_to_newest 1003426 0 mark.lam 2014-04-23 16:26:05 -0700 Currently this scenario can occur: 1. Thread 1 starts a GC and tries to suspend DFG worklist threads. However, no worklists were created yet at the that time. 2. Thread 2 starts to compile some functions and creates a DFG worklist, and acquires the worklist thread's lock. 3. Thread 1's GC completes and tries to resume suspended DFG worklist thread. This time, it sees the worklist created by Thread 2 and ends up unlocking the worklist thread's lock that is supposedly held by Thread 2. Thereafter, chaos ensues. The fix is to cache the worklists that were actually suspended by each GC pass, and only resume those when the GC is done. 1003427 1 webkit-bug-importer 2014-04-23 16:26:33 -0700 <rdar://problem/16706259> 1003433 2 230022 mark.lam 2014-04-23 16:34:40 -0700 Created attachment 230022 the patch 1003436 3 230022 mhahnenberg 2014-04-23 16:49:32 -0700 Comment on attachment 230022 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=230022&action=review r=me > Source/JavaScriptCore/heap/Heap.cpp:1025 > + ASSERT(!m_suspendedCompilerWorklists.size()); I find it's more readable to say "m_suspendedCompilerWorklists.isEmpty()" instead. > Source/JavaScriptCore/heap/Heap.cpp:1235 > + for (auto worklist : m_suspendedCompilerWorklists) > + worklist->resumeAllThreads(); > + m_suspendedCompilerWorklists.clear(); You should change the other places in the GC to only iterate this list. If the GC itself didn't suspend a work list then it shouldn't be touching that work list at all. 1003448 4 mark.lam 2014-04-23 17:44:23 -0700 Thanks. Suggested changes applied. Landed in r167733: <http://trac.webkit.org/r167733>. 230022 2014-04-23 16:34:40 -0700 2014-04-23 16:49:31 -0700 the patch bug-132088.patch text/plain 3507 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTY3NzI5KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDMxIEBA CisyMDE0LTA0LTIzICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBU aGUgR0Mgc2hvdWxkIG9ubHkgcmVzdW1lIGNvbXBpbGVyIHRocmVhZHMgdGhhdCBpdCBzdXNwZW5k ZWQgaW4gdGhlIHNhbWUgR0MgcGFzcy4KKyAgICAgICAgPGh0dHBzOi8vd2Via2l0Lm9yZy9iLzEz MjA4OD4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBQ cmV2aW91c2x5LCB0aGlzIHNjZW5hcmlvIGNhbiBvY2N1cjoKKyAgICAgICAgMS4gVGhyZWFkIDEg c3RhcnRzIGEgR0MgYW5kIHRyaWVzIHRvIHN1c3BlbmQgREZHIHdvcmtsaXN0IHRocmVhZHMuICBI b3dldmVyLAorICAgICAgICAgICBubyB3b3JrbGlzdHMgd2VyZSBjcmVhdGVkIHlldCBhdCB0aGUg dGhhdCB0aW1lLgorICAgICAgICAyLiBUaHJlYWQgMiBzdGFydHMgdG8gY29tcGlsZSBzb21lIGZ1 bmN0aW9ucyBhbmQgY3JlYXRlcyBhIERGRyB3b3JrbGlzdCwgYW5kCisgICAgICAgICAgIGFjcXVp cmVzIHRoZSB3b3JrbGlzdCB0aHJlYWQncyBsb2NrLgorICAgICAgICAzLiBUaHJlYWQgMSdzIEdD IGNvbXBsZXRlcyBhbmQgdHJpZXMgdG8gcmVzdW1lIHN1c3BlbmRlZCBERkcgd29ya2xpc3QgdGhy ZWFkLgorICAgICAgICAgICBUaGlzIHRpbWUsIGl0IHNlZXMgdGhlIHdvcmtsaXN0IGNyZWF0ZWQg YnkgVGhyZWFkIDIgYW5kIGVuZHMgdXAgdW5sb2NraW5nCisgICAgICAgICAgIHRoZSB3b3JrbGlz dCB0aHJlYWQncyBsb2NrIHRoYXQgaXMgc3VwcG9zZWRseSBoZWxkIGJ5IFRocmVhZCAyLgorICAg ICAgICBUaGVyZWFmdGVyLCBjaGFvcyBlbnN1ZXMuCisKKyAgICAgICAgVGhlIGZpeCBpcyB0byBj YWNoZSB0aGUgd29ya2xpc3RzIHRoYXQgd2VyZSBhY3R1YWxseSBzdXNwZW5kZWQgYnkgZWFjaCBH QyBwYXNzLAorICAgICAgICBhbmQgb25seSByZXN1bWUgdGhvc2Ugd2hlbiB0aGUgR0MgaXMgZG9u ZS4KKworICAgICAgICBUaGlzIGlzc3VlIHdhcyBkaXNjb3ZlcmVkIGJ5IGVuYWJsaW5nIENPTExF Q1RfT05fRVZFUllfQUxMT0NBVElPTiBhbmQgcnVubmluZworICAgICAgICB0aGUgZmFzdC93b3Jr ZXJzIGxheW91dCB0ZXN0cy4KKworICAgICAgICAqIGhlYXAvSGVhcC5jcHA6CisgICAgICAgIChK U0M6OkhlYXA6OnN1c3BlbmRDb21waWxlclRocmVhZHMpOgorICAgICAgICAoSlNDOjpIZWFwOjpy ZXN1bWVDb21waWxlclRocmVhZHMpOgorICAgICAgICAqIGhlYXAvSGVhcC5oOgorCiAyMDE0LTA0 LTIzICBNYXJrIEhhaG5lbmJlcmcgIDxtaGFobmVuYmVyZ0BhcHBsZS5jb20+CiAKICAgICAgICAg QXJndW1lbnRzOjpjb3B5QmFja2luZ1N0b3JlIG5lZWRzIHRvIHVwZGF0ZSBtX3JlZ2lzdGVycyBp biB0YW5kZW0gd2l0aCBtX3JlZ2lzdGVyQXJyYXkKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29y ZS9oZWFwL0hlYXAuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9oZWFw L0hlYXAuY3BwCShyZXZpc2lvbiAxNjc2OTEpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvaGVh cC9IZWFwLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTAyMiw5ICsxMDIyLDEyIEBAIHZvaWQgSGVh cDo6c3VzcGVuZENvbXBpbGVyVGhyZWFkcygpCiB7CiAjaWYgRU5BQkxFKERGR19KSVQpCiAgICAg R0NQSEFTRShTdXNwZW5kQ29tcGlsZXJUaHJlYWRzKTsKKyAgICBBU1NFUlQoIW1fc3VzcGVuZGVk Q29tcGlsZXJXb3JrbGlzdHMuc2l6ZSgpKTsKICAgICBmb3IgKHVuc2lnbmVkIGkgPSBERkc6Om51 bWJlck9mV29ya2xpc3RzKCk7IGktLTspIHsKLSAgICAgICAgaWYgKERGRzo6V29ya2xpc3QqIHdv cmtsaXN0ID0gREZHOjp3b3JrbGlzdEZvckluZGV4T3JOdWxsKGkpKQorICAgICAgICBpZiAoREZH OjpXb3JrbGlzdCogd29ya2xpc3QgPSBERkc6OndvcmtsaXN0Rm9ySW5kZXhPck51bGwoaSkpIHsK KyAgICAgICAgICAgIG1fc3VzcGVuZGVkQ29tcGlsZXJXb3JrbGlzdHMuYXBwZW5kKHdvcmtsaXN0 KTsKICAgICAgICAgICAgIHdvcmtsaXN0LT5zdXNwZW5kQWxsVGhyZWFkcygpOworICAgICAgICB9 CiAgICAgfQogI2VuZGlmCiB9CkBAIC0xMjI3LDEwICsxMjMwLDkgQEAgdm9pZCBIZWFwOjpyZXN1 bWVDb21waWxlclRocmVhZHMoKQogewogI2lmIEVOQUJMRShERkdfSklUKQogICAgIEdDUEhBU0Uo UmVzdW1lQ29tcGlsZXJUaHJlYWRzKTsKLSAgICBmb3IgKHVuc2lnbmVkIGkgPSBERkc6Om51bWJl ck9mV29ya2xpc3RzKCk7IGktLTspIHsKLSAgICAgICAgaWYgKERGRzo6V29ya2xpc3QqIHdvcmts aXN0ID0gREZHOjp3b3JrbGlzdEZvckluZGV4T3JOdWxsKGkpKQotICAgICAgICAgICAgd29ya2xp c3QtPnJlc3VtZUFsbFRocmVhZHMoKTsKLSAgICB9CisgICAgZm9yIChhdXRvIHdvcmtsaXN0IDog bV9zdXNwZW5kZWRDb21waWxlcldvcmtsaXN0cykKKyAgICAgICAgd29ya2xpc3QtPnJlc3VtZUFs bFRocmVhZHMoKTsKKyAgICBtX3N1c3BlbmRlZENvbXBpbGVyV29ya2xpc3RzLmNsZWFyKCk7CiAj ZW5kaWYKIH0KIApJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2hlYXAvSGVhcC5oCj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9oZWFwL0hlYXAuaAkocmV2aXNpb24gMTY3 NjkxKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2hlYXAvSGVhcC5oCSh3b3JraW5nIGNvcHkp CkBAIC03MSw2ICs3MSwxMCBAQCBjbGFzcyBNYXJrZWRBcmd1bWVudEJ1ZmZlcjsKIGNsYXNzIFdl YWtHQ0hhbmRsZVBvb2w7CiBjbGFzcyBTbG90VmlzaXRvcjsKIAorbmFtZXNwYWNlIERGRyB7Citj bGFzcyBXb3JrbGlzdDsKK30KKwogdHlwZWRlZiBzdGQ6OnBhaXI8SlNWYWx1ZSwgV1RGOjpTdHJp bmc+IFZhbHVlU3RyaW5nUGFpcjsKIHR5cGVkZWYgSGFzaENvdW50ZWRTZXQ8SlNDZWxsKj4gUHJv dGVjdENvdW50U2V0OwogdHlwZWRlZiBIYXNoQ291bnRlZFNldDxjb25zdCBjaGFyKj4gVHlwZUNv dW50U2V0OwpAQCAtMzczLDYgKzM3Nyw3IEBAIHByaXZhdGU6CiAgICAgVmVjdG9yPE1hcmtlZEJs b2NrKj4gbV9ibG9ja1NuYXBzaG90OwogICAgIAogICAgIHVuc2lnbmVkIG1fZGVmZXJyYWxEZXB0 aDsKKyAgICBWZWN0b3I8REZHOjpXb3JrbGlzdCo+IG1fc3VzcGVuZGVkQ29tcGlsZXJXb3JrbGlz dHM7CiB9OwogCiB9IC8vIG5hbWVzcGFjZSBKU0MK