132144 2014-04-24 14:11:06 -0700 Don't use DispatchMessageEvenWhenWaitingForSyncReply for messages from NetworkProcess 2014-04-27 18:56:06 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 ap ap andersca beidson commit-queue sam oldest_to_newest 1003771 0 ap 2014-04-24 14:11:06 -0700 void AsynchronousNetworkLoaderClient::willSendRequest(NetworkResourceLoader* loader, ResourceRequest& request, const ResourceResponse& redirectResponse) { // This message is DispatchMessageEvenWhenWaitingForSyncReply to avoid a situation where the NetworkProcess is deadlocked // waiting for 6 connections to complete while the WebProcess is waiting for a 7th (Synchronous XHR) to complete. loader->sendAbortingOnFailure(Messages::WebResourceLoader::WillSendRequest(request, redirectResponse), IPC::DispatchMessageEvenWhenWaitingForSyncReply); } There are several confusing things here: 1. If we send messages with IPC::DispatchMessageEvenWhenWaitingForSyncReply from NetworkProcess, they can be handled during sync XHR, causing undesirable reentrancy. That's dangerous. 2. But IPC::DispatchMessageEvenWhenWaitingForSyncReply doesn't really work in NetworkProcess, because the connection uses setOnlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage(true), so the flag is usually ignored. When it's not ignored, see the above. 3. The comment claims that we need this because otherwise we would have a deadlock. It's not accurate - we would happily load sync requests even if there are 6 async ones stuck. We disable connection limits for sync requests, even when pipelining is not in use. This is probably difficult to reproduce in practice. 1003772 1 230105 ap 2014-04-24 14:13:49 -0700 Created attachment 230105 proposed patch 1004459 2 230105 commit-queue 2014-04-27 18:56:03 -0700 Comment on attachment 230105 proposed patch Clearing flags on attachment: 230105 Committed r167866: <http://trac.webkit.org/changeset/167866> 1004460 3 commit-queue 2014-04-27 18:56:06 -0700 All reviewed patches have been landed. Closing bug. 230105 2014-04-24 14:13:49 -0700 2014-04-27 18:56:03 -0700 proposed patch NetworkProcessSyncMessaging.txt text/plain 4738 ap SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE2Nzc3MSkKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI0IEBACisyMDE0LTA0LTI0ICBBbGV4ZXkg UHJvc2t1cnlha292ICA8YXBAYXBwbGUuY29tPgorCisgICAgICAgIERvbid0IHVzZSBEaXNwYXRj aE1lc3NhZ2VFdmVuV2hlbldhaXRpbmdGb3JTeW5jUmVwbHkgZm9yIG1lc3NhZ2VzIGZyb20gTmV0 d29ya1Byb2Nlc3MKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTEzMjE0NAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgICogTmV0d29ya1Byb2Nlc3MvQXN5bmNocm9ub3VzTmV0d29ya0xvYWRlckNsaWVudC5jcHA6 CisgICAgICAgIChXZWJLaXQ6OkFzeW5jaHJvbm91c05ldHdvcmtMb2FkZXJDbGllbnQ6OndpbGxT ZW5kUmVxdWVzdCk6CisgICAgICAgIChXZWJLaXQ6OkFzeW5jaHJvbm91c05ldHdvcmtMb2FkZXJD bGllbnQ6OmNhbkF1dGhlbnRpY2F0ZUFnYWluc3RQcm90ZWN0aW9uU3BhY2UpOgorICAgICAgICBE b250IHVzZSB0aGUgSVBDOjpEaXNwYXRjaE1lc3NhZ2VFdmVuV2hlbldhaXRpbmdGb3JTeW5jUmVw bHkgZmxhZy4gSXQncyBub3QgbmVlZGVkLAorICAgICAgICBhbG1vc3QgbmV2ZXIgd29ya3MgaW4g TmV0d29ya1Byb2Nlc3MsIGJ1dCBtYXkgY2F1c2UgdHJvdWJsZSBpZiBpdCBkaWQuCisKKyAgICAg ICAgKiBOZXR3b3JrUHJvY2Vzcy9OZXR3b3JrQ29ubmVjdGlvblRvV2ViUHJvY2Vzcy5jcHA6Cisg ICAgICAgIChXZWJLaXQ6Ok5ldHdvcmtDb25uZWN0aW9uVG9XZWJQcm9jZXNzOjpOZXR3b3JrQ29u bmVjdGlvblRvV2ViUHJvY2Vzcyk6IFJlbW92ZWQKKyAgICAgICAgc2V0T25seVNlbmRNZXNzYWdl c0FzRGlzcGF0Y2hXaGVuV2FpdGluZ0ZvclN5bmNSZXBseVdoZW5Qcm9jZXNzaW5nU3VjaEFNZXNz YWdlKCkKKyAgICAgICAgZnVuY3Rpb24gY2FsbCwgYmVjYXVzZSB0aGVyZSBhcmUgbm8gbW9yZSBh bnkgbWVzc2FnZXMgaW4gTmV0d29ya1Byb2Nlc3MgaXQgY291bGQgYWZmZWN0LgorCisgICAgICAg ICogUGxhdGZvcm0vSVBDL0Nvbm5lY3Rpb24uY3BwOiAoSVBDOjpDb25uZWN0aW9uOjpkaXNwYXRj aE1lc3NhZ2UpOiBBZGRlZCBhIEZJWE1FCisgICAgICAgIGZvciBhbiB1bnJlbGF0ZWQgYnVnIEFu ZGVycyBhbmQgbXlzZWxmIG5vdGljZWQgd2hpbGUgbG9va2luZyBpbnRvIHRoaXMuCisKIDIwMTQt MDQtMjQgIE15bGVzIEMuIE1heGZpZWxkICA8bW1heGZpZWxkQGFwcGxlLmNvbT4KIAogICAgICAg ICBGb250Q2FjaGU6OmZvbnRDYWNoZSgpIG5ldmVyIHJldHVybnMgbnVsbHB0ciBzbyBpdCBjYW4g YmUgbWFkZSB0byByZXR1cm4gYSByZWZlcmVuY2UgaW5zdGVhZApJbmRleDogU291cmNlL1dlYktp dDIvTmV0d29ya1Byb2Nlc3MvQXN5bmNocm9ub3VzTmV0d29ya0xvYWRlckNsaWVudC5jcHAKPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gU291cmNlL1dlYktpdDIvTmV0d29ya1Byb2Nlc3MvQXN5bmNocm9ub3VzTmV0 d29ya0xvYWRlckNsaWVudC5jcHAJKHJldmlzaW9uIDE2NzUwMCkKKysrIFNvdXJjZS9XZWJLaXQy L05ldHdvcmtQcm9jZXNzL0FzeW5jaHJvbm91c05ldHdvcmtMb2FkZXJDbGllbnQuY3BwCSh3b3Jr aW5nIGNvcHkpCkBAIC00NywxNyArNDcsMTMgQEAgQXN5bmNocm9ub3VzTmV0d29ya0xvYWRlckNs aWVudDo6QXN5bmNocgogCiB2b2lkIEFzeW5jaHJvbm91c05ldHdvcmtMb2FkZXJDbGllbnQ6Ondp bGxTZW5kUmVxdWVzdChOZXR3b3JrUmVzb3VyY2VMb2FkZXIqIGxvYWRlciwgUmVzb3VyY2VSZXF1 ZXN0JiByZXF1ZXN0LCBjb25zdCBSZXNvdXJjZVJlc3BvbnNlJiByZWRpcmVjdFJlc3BvbnNlKQog ewotICAgIC8vIFRoaXMgbWVzc2FnZSBpcyBEaXNwYXRjaE1lc3NhZ2VFdmVuV2hlbldhaXRpbmdG b3JTeW5jUmVwbHkgdG8gYXZvaWQgYSBzaXR1YXRpb24gd2hlcmUgdGhlIE5ldHdvcmtQcm9jZXNz IGlzIGRlYWRsb2NrZWQKLSAgICAvLyB3YWl0aW5nIGZvciA2IGNvbm5lY3Rpb25zIHRvIGNvbXBs ZXRlIHdoaWxlIHRoZSBXZWJQcm9jZXNzIGlzIHdhaXRpbmcgZm9yIGEgN3RoIChTeW5jaHJvbm91 cyBYSFIpIHRvIGNvbXBsZXRlLgotICAgIGxvYWRlci0+c2VuZEFib3J0aW5nT25GYWlsdXJlKE1l c3NhZ2VzOjpXZWJSZXNvdXJjZUxvYWRlcjo6V2lsbFNlbmRSZXF1ZXN0KHJlcXVlc3QsIHJlZGly ZWN0UmVzcG9uc2UpLCBJUEM6OkRpc3BhdGNoTWVzc2FnZUV2ZW5XaGVuV2FpdGluZ0ZvclN5bmNS ZXBseSk7CisgICAgbG9hZGVyLT5zZW5kQWJvcnRpbmdPbkZhaWx1cmUoTWVzc2FnZXM6OldlYlJl c291cmNlTG9hZGVyOjpXaWxsU2VuZFJlcXVlc3QocmVxdWVzdCwgcmVkaXJlY3RSZXNwb25zZSkp OwogfQogCiAjaWYgVVNFKFBST1RFQ1RJT05fU1BBQ0VfQVVUSF9DQUxMQkFDSykKIHZvaWQgQXN5 bmNocm9ub3VzTmV0d29ya0xvYWRlckNsaWVudDo6Y2FuQXV0aGVudGljYXRlQWdhaW5zdFByb3Rl Y3Rpb25TcGFjZShOZXR3b3JrUmVzb3VyY2VMb2FkZXIqIGxvYWRlciwgY29uc3QgUHJvdGVjdGlv blNwYWNlJiBwcm90ZWN0aW9uU3BhY2UpCiB7Ci0gICAgLy8gVGhpcyBtZXNzYWdlIGlzIERpc3Bh dGNoTWVzc2FnZUV2ZW5XaGVuV2FpdGluZ0ZvclN5bmNSZXBseSB0byBhdm9pZCBhIHNpdHVhdGlv biB3aGVyZSB0aGUgTmV0d29ya1Byb2Nlc3MgaXMgZGVhZGxvY2tlZAotICAgIC8vIHdhaXRpbmcg Zm9yIDYgY29ubmVjdGlvbnMgdG8gY29tcGxldGUgd2hpbGUgdGhlIFdlYlByb2Nlc3MgaXMgd2Fp dGluZyBmb3IgYSA3dGggKFN5bmNocm9ub3VzIFhIUikgdG8gY29tcGxldGUuCi0gICAgbG9hZGVy LT5zZW5kQWJvcnRpbmdPbkZhaWx1cmUoTWVzc2FnZXM6OldlYlJlc291cmNlTG9hZGVyOjpDYW5B dXRoZW50aWNhdGVBZ2FpbnN0UHJvdGVjdGlvblNwYWNlKHByb3RlY3Rpb25TcGFjZSksIElQQzo6 RGlzcGF0Y2hNZXNzYWdlRXZlbldoZW5XYWl0aW5nRm9yU3luY1JlcGx5KTsKKyAgICBsb2FkZXIt PnNlbmRBYm9ydGluZ09uRmFpbHVyZShNZXNzYWdlczo6V2ViUmVzb3VyY2VMb2FkZXI6OkNhbkF1 dGhlbnRpY2F0ZUFnYWluc3RQcm90ZWN0aW9uU3BhY2UocHJvdGVjdGlvblNwYWNlKSk7CiB9CiAj ZW5kaWYKIApJbmRleDogU291cmNlL1dlYktpdDIvTmV0d29ya1Byb2Nlc3MvTmV0d29ya0Nvbm5l Y3Rpb25Ub1dlYlByb2Nlc3MuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQyL05ldHdv cmtQcm9jZXNzL05ldHdvcmtDb25uZWN0aW9uVG9XZWJQcm9jZXNzLmNwcAkocmV2aXNpb24gMTY3 NTAwKQorKysgU291cmNlL1dlYktpdDIvTmV0d29ya1Byb2Nlc3MvTmV0d29ya0Nvbm5lY3Rpb25U b1dlYlByb2Nlc3MuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC01OCw3ICs1OCw2IEBAIE5ldHdvcmtD b25uZWN0aW9uVG9XZWJQcm9jZXNzOjpOZXR3b3JrQ28KICAgICA6IG1fc2VyaWFsTG9hZGluZ0Vu YWJsZWQoZmFsc2UpCiB7CiAgICAgbV9jb25uZWN0aW9uID0gSVBDOjpDb25uZWN0aW9uOjpjcmVh dGVTZXJ2ZXJDb25uZWN0aW9uKGNvbm5lY3Rpb25JZGVudGlmaWVyLCB0aGlzLCBSdW5Mb29wOjpt YWluKCkpOwotICAgIG1fY29ubmVjdGlvbi0+c2V0T25seVNlbmRNZXNzYWdlc0FzRGlzcGF0Y2hX aGVuV2FpdGluZ0ZvclN5bmNSZXBseVdoZW5Qcm9jZXNzaW5nU3VjaEFNZXNzYWdlKHRydWUpOwog ICAgIG1fY29ubmVjdGlvbi0+b3BlbigpOwogfQogCkluZGV4OiBTb3VyY2UvV2ViS2l0Mi9QbGF0 Zm9ybS9JUEMvQ29ubmVjdGlvbi5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYktpdDIvUGxh dGZvcm0vSVBDL0Nvbm5lY3Rpb24uY3BwCShyZXZpc2lvbiAxNjc1MDApCisrKyBTb3VyY2UvV2Vi S2l0Mi9QbGF0Zm9ybS9JUEMvQ29ubmVjdGlvbi5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTc5Myw2 ICs3OTMsOCBAQCB2b2lkIENvbm5lY3Rpb246OmRpc3BhdGNoTWVzc2FnZShzdGQ6OnVuCiAgICAg bV9kaWRSZWNlaXZlSW52YWxpZE1lc3NhZ2UgfD0gbWVzc2FnZS0+aXNJbnZhbGlkKCk7CiAgICAg bV9pbkRpc3BhdGNoTWVzc2FnZUNvdW50LS07CiAKKyAgICAvLyBGSVhNRTogRm9yIERlbGF5ZWQg c3luY2hyb25vdXMgbWVzc2FnZXMsIHdlIHNob3VsZCBub3QgZGVjcmVtZW50IHRoZSBjb3VudGVy IHVudGlsIHdlIHNlbmQgYSByZXNwb25zZS4KKyAgICAvLyBPdGhlcndpc2UsIHdlIHdvdWxkIGRl YWRsb2NrIGlmIHByb2Nlc3NpbmcgdGhlIG1lc3NhZ2UgcmVzdWx0cyBpbiBhIHN5bmMgbWVzc2Fn ZSBiYWNrIGFmdGVyIHdlIGV4aXQgdGhpcyBmdW5jdGlvbi4KICAgICBpZiAobWVzc2FnZS0+c2hv dWxkRGlzcGF0Y2hNZXNzYWdlV2hlbldhaXRpbmdGb3JTeW5jUmVwbHkoKSkKICAgICAgICAgbV9p bkRpc3BhdGNoTWVzc2FnZU1hcmtlZERpc3BhdGNoV2hlbldhaXRpbmdGb3JTeW5jUmVwbHlDb3Vu dC0tOwogCg==