159331 2016-06-30 23:14:01 -0700 Deadlock inside -[WebCoreNSURLSession dealloc] 2016-07-01 10:24:16 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 jer.noble jer.noble benjamin cdumez cmarcelo commit-queue ggaren webkit-bug-importer oldest_to_newest 1206996 0 jer.noble 2016-06-30 23:14:01 -0700 SPIN inside -[WebCoreNSURLSession dealloc] 1206999 1 jer.noble 2016-06-30 23:21:51 -0700 <rdar://problem/27122716> 1207000 2 282513 jer.noble 2016-06-30 23:25:41 -0700 Created attachment 282513 Patch 1207011 3 282513 achristensen 2016-07-01 00:20:20 -0700 Comment on attachment 282513 Patch Alternatively, you could just move the declaration of function inside the while loop scope (but not in the lock scope). 1207125 4 jer.noble 2016-07-01 08:38:03 -0700 (In reply to comment #3) > Comment on attachment 282513 [details] > Patch > > Alternatively, you could just move the declaration of function inside the > while loop scope (but not in the lock scope). I considered that, but thought this way would be slightly less expensive (no call to the constructor and destructor), as well as is more explicit about what's going on. 1207127 5 282513 commit-queue 2016-07-01 08:41:05 -0700 Comment on attachment 282513 Patch Clearing flags on attachment: 282513 Committed r202736: <http://trac.webkit.org/changeset/202736> 1207128 6 commit-queue 2016-07-01 08:41:10 -0700 All reviewed patches have been landed. Closing bug. 1207143 7 282513 ggaren 2016-07-01 09:30:18 -0700 Comment on attachment 282513 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=282513&action=review > Source/WTF/wtf/MainThread.cpp:137 > + // Clearing the function can have side effects, so do so outside of the lock above. > + function = nullptr; Can we just move the declaration of function inside the loop? That's the C++ way to specify the semantics you want: Don't allow this value to outlive this loop body. 1207152 8 ggaren 2016-07-01 09:34:54 -0700 > > Alternatively, you could just move the declaration of function inside the > > while loop scope (but not in the lock scope). > > > I considered that, but thought this way would be slightly less expensive (no > call to the constructor and destructor), as well as is more explicit about > what's going on. There's no performance concern here. The empty constructor just assigns nullptr, which is exactly what you've done manually. Actually, there's a slight performance win, since it's much easier for the compiler to notice that nullptr is never observed, and eliminate the store entirely. 1207188 9 jer.noble 2016-07-01 10:24:16 -0700 (In reply to comment #8) > > > Alternatively, you could just move the declaration of function inside the > > > while loop scope (but not in the lock scope). > > > > > > I considered that, but thought this way would be slightly less expensive (no > > call to the constructor and destructor), as well as is more explicit about > > what's going on. > > There's no performance concern here. The empty constructor just assigns > nullptr, which is exactly what you've done manually. Actually, there's a > slight performance win, since it's much easier for the compiler to notice > that nullptr is never observed, and eliminate the store entirely. Okay, sure. I'll post a follow up. 282513 2016-06-30 23:25:41 -0700 2016-07-01 08:41:05 -0700 Patch bug-159331-20160630232519.patch text/plain 2132 jer.noble U3VidmVyc2lvbiBSZXZpc2lvbjogMjAyNzE5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IDZiZDM0M2Y0OTg0MTVlMzIxZTY1ODQ5 ZGE1MDI2ZDJiNGRiYzRiN2UuLjFiM2VmYmRjMzJkMWRiMTYyODA1ODcxMWQ4NzRlYTgwNjRiYmFh ODcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMjUgQEAKKzIwMTYtMDYtMzAgIEplciBOb2JsZSAgPGplci5ub2Js ZUBhcHBsZS5jb20+CisKKyAgICAgICAgRGVhZGxvY2sgaW5zaWRlIC1bV2ViQ29yZU5TVVJMU2Vz c2lvbiBkZWFsbG9jXQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MTU5MzMxCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8yNzEyMjcxNj4KKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBBIEZ1bmN0aW9uPD4gb2Jq ZWN0IGNhbiB3cmFwIGFueSBjYWxsYWJsZSB0eXBlLCBpbmNsdWRpbmcgYSBDKysgbGFtYmRhLgor CisgICAgICAgIGRpc3BhdGNoRnVuY3Rpb25zRnJvbU1haW5UaHJlYWQoKSBob2xkcyBhIGxvY2sg d2hpbGUgaXRlcmF0aW5nIG92ZXIgdGhlIGZ1bmN0aW9ucyBpbgorICAgICAgICBmdW5jdGlvblF1 ZXVlKCksIGFuZCBkdXJpbmcgdGhzIGl0ZXJhdGlvbiwgdGhlIHByZXZpb3VzIGNhbGxhYmxlIG9i amVjdCBpcyBkZXN0cm95ZWQgYnkKKyAgICAgICAgYXNzaWduaW5nIHRoZSByZXN1bHQgb2YgZnVu Y3Rpb25RdWV1ZSgpLnRha2VGaXJzdCgpLiBCZWNhdXNlIGxhbWJkYXMgKGFuZCBvdGhlciBjYWxs YWJsZXMsCisgICAgICAgIGxpa2UgZnVuY3RvcnMpIGNhbiBvd24gb2JqZWN0cywgZGVzdHJveWlu ZyB0aGlzIGNhbGxhYmxlIGNhbiBoYXZlIHNpZGUgZWZmZWN0cywgYW5kIGlmIG9uZQorICAgICAg ICBvZiB0aG9zZSBzaWRlIGVmZmVjdHMgaXMgdG8gY2FsbCBjYWxsT25NYWluVGhyZWFkKCksIHRo aXMgY2FuIGRlYWRsb2NrLgorCisgICAgICAgIE1vdmUgdGhpcyBzaWRlLWVmZmVjdC1oYXZpbmcg Y2FsbCBvdXRzaWRlIHRoZSBsb2NrZWQgYmxvY2sgYnkgY2xlYXJpbmcgdGhlIGZ1bmN0aW9uIG9i amVjdAorICAgICAgICBpbW1lZGlhdGVseSBhZnRlciBjYWxsaW5nIGl0LgorCisgICAgICAgICog d3RmL01haW5UaHJlYWQuY3BwOgorICAgICAgICAoV1RGOjpkaXNwYXRjaEZ1bmN0aW9uc0Zyb21N YWluVGhyZWFkKToKKwogMjAxNi0wNi0yOSAgSmVyIE5vYmxlICA8amVyLm5vYmxlQGFwcGxlLmNv bT4KIAogICAgICAgICBBZG9wdCBNZWRpYVJlbW90ZS4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XVEYv d3RmL01haW5UaHJlYWQuY3BwIGIvU291cmNlL1dURi93dGYvTWFpblRocmVhZC5jcHAKaW5kZXgg NmQ0MmM0ZmRiZGFkZTJiMDE3ODEzYmE0NDcwZGQyY2RkYWE3NWY4YS4uYTlkNDZhOWI5NjE0YWU4 YWFiODMyMWU2ZjY2MGI0MTk5MjIxMjE5YyAxMDA2NDQKLS0tIGEvU291cmNlL1dURi93dGYvTWFp blRocmVhZC5jcHAKKysrIGIvU291cmNlL1dURi93dGYvTWFpblRocmVhZC5jcHAKQEAgLTEzMyw2 ICsxMzMsOSBAQCB2b2lkIGRpc3BhdGNoRnVuY3Rpb25zRnJvbU1haW5UaHJlYWQoKQogCiAgICAg ICAgIGZ1bmN0aW9uKCk7CiAKKyAgICAgICAgLy8gQ2xlYXJpbmcgdGhlIGZ1bmN0aW9uIGNhbiBo YXZlIHNpZGUgZWZmZWN0cywgc28gZG8gc28gb3V0c2lkZSBvZiB0aGUgbG9jayBhYm92ZS4KKyAg ICAgICAgZnVuY3Rpb24gPSBudWxscHRyOworCiAgICAgICAgIC8vIElmIHdlIGFyZSBydW5uaW5n IGFjY3VtdWxhdGVkIGZ1bmN0aW9ucyBmb3IgdG9vIGxvbmcgc28gVUkgbWF5IGJlY29tZSB1bnJl c3BvbnNpdmUsIHdlIG5lZWQgdG8KICAgICAgICAgLy8geWllbGQgc28gdGhlIHVzZXIgaW5wdXQg Y2FuIGJlIHByb2Nlc3NlZC4gT3RoZXJ3aXNlIHVzZXIgbWF5IG5vdCBiZSBhYmxlIHRvIGV2ZW4g Y2xvc2UgdGhlIHdpbmRvdy4KICAgICAgICAgLy8gVGhpcyBjb2RlIGhhcyBlZmZlY3Qgb25seSBp biBjYXNlIHRoZSBzY2hlZHVsZURpc3BhdGNoRnVuY3Rpb25zT25NYWluVGhyZWFkKCkgaXMgaW1w bGVtZW50ZWQgaW4gYSB3YXkgdGhhdAo=