71122 2011-10-28 09:06:31 -0700 properly end requests when a bad status code return happens 2011-11-01 11:55:11 -0700 1 1 1 Unclassified WebKit Page Loading 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 gavinp gavinp ap japhet koivisto webkit.review.bot oldest_to_newest 492480 0 gavinp 2011-10-28 09:06:31 -0700 properly end requests when a bad status code return happens 492481 1 112873 gavinp 2011-10-28 09:18:45 -0700 Created attachment 112873 Patch 492482 2 gavinp 2011-10-28 09:22:21 -0700 Calling error without ending the request set up the CachedResourceRequest so that it could actually send out two notifyFinished() events. This probably was the root cause of lots of crashing instability; I know from crbug.com/75604 that this bug was causing lots of crashes in ScriptRunner/ScriptElement for instance. The fix is easy: just properly end the request instead of just calling error, and we won't re-notify. I'm in the process of landing a test for this in chromium first; by instrumenting Chromium's network stack, I was able to create a crash on every load, see http://codereview.chromium.org/8404001/ for the ongoing review of that test. My plan is: 1. Land that chromium test with a crash expectation. 2. Remove the crash expectation in the chromium test. 3. Land this change. 4. Add a doesn't-crash expectation to the chromium test. I tried to create a Layout test for this, but it's not really possible to do in Lighthttpd. I can do a reproduction of this problem with Chromium's network stack using a python based web server. I am not sure the crash I'm getting would reproduce in a WebKit port that has an in-process network stack. 493849 3 gavinp 2011-11-01 04:56:56 -0700 japhet: *ping* ? 493920 4 112873 japhet 2011-11-01 08:55:57 -0700 Comment on attachment 112873 Patch Code looks fine. I assume the logging code you add to diagnose this will be removed in a separate patch? Is there any hope of a manual test for this? I'm guessing not, since it requires doing crazy things with the network? 493956 5 113178 gavinp 2011-11-01 09:43:42 -0700 Created attachment 113178 python script to replicate bug in chrome This script can be used for manual testing in chrome. It runs as a web server on port 9095, so run this script, and navigate to http://localhost:9095/ locally to see the replication. 493957 6 113179 gavinp 2011-11-01 09:44:48 -0700 Created attachment 113179 alternative reproduction This script also operates as a web server, on port 9096. Run this script, and navigate to http://localhost:9096/ to replicate this bug. 493959 7 gavinp 2011-11-01 09:46:41 -0700 Thanks Nate. I've attached two python scripts that can help reproduce this; they act as web servers, and yes, they do some funny network juju. I wish there was a good way to test this that fits into our manual testing, but I spent some effort and couldn't. Is there a mechanism in lighty to grab the socket for a CGI? If so, the ABORT reproduction (continue.py above) would be possible in a layout test, and I'd like that. Otherwise, I'll land this soon, and enable a browser_test in chromium to look for regressions. In a separate CL next week I'll remove all my instrumentation, if this crash actually goes away in the wild after my test is uploaded. 494053 8 112873 webkit.review.bot 2011-11-01 11:55:07 -0700 Comment on attachment 112873 Patch Clearing flags on attachment: 112873 Committed r98987: <http://trac.webkit.org/changeset/98987> 494054 9 webkit.review.bot 2011-11-01 11:55:11 -0700 All reviewed patches have been landed. Closing bug. 112873 2011-10-28 09:18:45 -0700 2011-11-01 11:55:07 -0700 Patch bug-71122-20111028121844.patch text/plain 2202 gavinp U3VidmVyc2lvbiBSZXZpc2lvbjogOTg1MzcKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCBjZTJlMjE0OGNiODE5ZGEz NzM0MzhiYTk3NTlhMzNjOGMzOTZmNzhjLi5iMDljOWQ4N2JlNjg0Mzc4MjhjNTg4MzQ1MmYyZTVm N2JlMDI5NTg0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjQgQEAKKzIwMTEtMTAtMjggIEdhdmlu IFBldGVycyAgPGdhdmlucEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgcHJvcGVybHkgZW5kIHJl cXVlc3RzIHdoZW4gYSBiYWQgc3RhdHVzIGNvZGUgcmV0dXJuIGhhcHBlbnMKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTcxMTIyCisKKyAgICAgICAgQ2Fs bGluZyBlcnJvciB3aXRob3V0IGVuZGluZyB0aGUgcmVxdWVzdCBzZXQgdXAgdGhlIENhY2hlZFJl c291cmNlUmVxdWVzdCBzbyB0aGF0IGl0IGNvdWxkCisgICAgICAgIGFjdHVhbGx5IHNlbmQgb3V0 IHR3byBub3RpZnlGaW5pc2hlZCgpIGV2ZW50cy4gIFRoaXMgcHJvYmFibHkgd2FzIHRoZSByb290 IGNhdXNlIG9mCisgICAgICAgIGxvdHMgb2YgY3Jhc2hpbmcgaW5zdGFiaWxpdHk7IEkga25vdyBm cm9tIGNyYnVnLmNvbS83NTYwNCB0aGF0IHRoaXMgYnVnIHdhcyBjYXVzaW5nIGxvdHMKKyAgICAg ICAgb2YgY3Jhc2hlcyBpbiBTY3JpcHRSdW5uZXIvU2NyaXB0RWxlbWVudCBmb3IgaW5zdGFuY2Uu CisKKyAgICAgICAgVGhlIGZpeCBpcyBlYXN5OiBqdXN0IHByb3Blcmx5IGVuZCB0aGUgcmVxdWVz dCBpbnN0ZWFkIG9mIGp1c3QgY2FsbGluZyBlcnJvciwgYW5kIHdlIHdvbid0CisgICAgICAgIHJl LW5vdGlmeS4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBObyBuZXcgdGVzdHMsIGFzIHRoZSBwcm9ibGVtIHdhc24ndCB2ZXJ5IGFtZW5hYmxlIHRvIGxh eW91dCB0ZXN0cy4KKyAgICAgICAgVGhlcmUgaXMgYSBjaHJvbWl1bSB0ZXN0IGdvaW5nIHRocm91 Z2ggY29kZSByZXZpZXcgYXQgaHR0cDovL2NvZGVyZXZpZXcuY2hyb21pdW0ub3JnLzg0MDQwMDEv CisKKyAgICAgICAgKiBsb2FkZXIvY2FjaGUvQ2FjaGVkUmVzb3VyY2VSZXF1ZXN0LmNwcDoKKyAg ICAgICAgKFdlYkNvcmU6OkNhY2hlZFJlc291cmNlUmVxdWVzdDo6ZGlkUmVjZWl2ZURhdGEpOgor CiAyMDExLTEwLTI2ICBTdGV2ZSBGYWxrZW5idXJnICA8c2ZhbGtlbkBhcHBsZS5jb20+CiAKICAg ICAgICAgUmVzb3VyY2VFcnJvciBkb2Vzbid0IHN1cHBvcnQgY3VzdG9tIGRvbWFpbnMgb24gV2lu ZG93cwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvbG9hZGVyL2NhY2hlL0NhY2hlZFJlc291 cmNlUmVxdWVzdC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvY2FjaGUvQ2FjaGVkUmVzb3Vy Y2VSZXF1ZXN0LmNwcAppbmRleCA0NDY2YWRjMjg0NDhlNWFlNTQ2YTc2ZWMzODdjOTU2MWY5ODdk NGM3Li4zNzNkMzUxYTEzNTBhOTg0NjM3NjUyODhlZjUwYTZlZWJiNGZhMGEwIDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvY2FjaGUvQ2FjaGVkUmVzb3VyY2VSZXF1ZXN0LmNwcAor KysgYi9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvY2FjaGUvQ2FjaGVkUmVzb3VyY2VSZXF1ZXN0LmNw cApAQCAtMjkyLDcgKzI5MiwxMiBAQCB2b2lkIENhY2hlZFJlc291cmNlUmVxdWVzdDo6ZGlkUmVj ZWl2ZURhdGEoU3VicmVzb3VyY2VMb2FkZXIqIGxvYWRlciwgY29uc3QgY2hhcgogICAgICAgICBy ZXR1cm47CiAKICAgICBpZiAobV9yZXNvdXJjZS0+cmVzcG9uc2UoKS5odHRwU3RhdHVzQ29kZSgp ID49IDQwMCAmJiAhbV9yZXNvdXJjZS0+c2hvdWxkSWdub3JlSFRUUFN0YXR1c0NvZGVFcnJvcnMo KSkgeworICAgICAgICBpZiAoIW1fbXVsdGlwYXJ0KQorICAgICAgICAgICAgbV9jYWNoZWRSZXNv dXJjZUxvYWRlci0+ZGVjcmVtZW50UmVxdWVzdENvdW50KG1fcmVzb3VyY2UpOworICAgICAgICBt X2ZpbmlzaGluZyA9IHRydWU7CisgICAgICAgIG1fbG9hZGVyLT5jbGVhckNsaWVudCgpOwogICAg ICAgICBtX3Jlc291cmNlLT5lcnJvcihDYWNoZWRSZXNvdXJjZTo6TG9hZEVycm9yKTsKKyAgICAg ICAgZW5kKCk7CiAgICAgICAgIHJldHVybjsKICAgICB9CiAK 113178 2011-11-01 09:43:42 -0700 2011-11-01 09:43:42 -0700 python script to replicate bug in chrome continue.py text/x-python-script 1430 gavinp IyEvdXNyL2Jpbi9weXRob24KCmltcG9ydCB0aW1lCmltcG9ydCBzb2NrZXQKaW1wb3J0IHN0cnVj dAppbXBvcnQgQmFzZUhUVFBTZXJ2ZXIKCgpIT1NUX05BTUUgPSAnMC4wLjAuMCcgClBPUlRfTlVN QkVSID0gOTA5NQoKY2xhc3MgTXlIYW5kbGVyKEJhc2VIVFRQU2VydmVyLkJhc2VIVFRQUmVxdWVz dEhhbmRsZXIpOgogICAgZGVmIGRvX0dFVChzKToKICAgICAgICAiIiJSZXNwb25kIHRvIGEgR0VU IHJlcXVlc3QuIiIiCiAgICAgICAgaWYgcy5wYXRoID09ICIvIjoKICAgICAgICAgICAgcy5zZW5k X3Jlc3BvbnNlKDIwMCk7CiAgICAgICAgICAgIHMuZW5kX2hlYWRlcnMoKTsKICAgICAgICAgICAg cy53ZmlsZS53cml0ZSgiIiIKPGh0bWw+PGhlYWQ+PHNjcmlwdCBhc3luYyBzcmM9InNjcmlwdC5q cyI+PC9zY3JpcHQ+PC9oZWFkPgo8Ym9keT50ZXN0IGRvY3VtZW50IHRoYXQgc2hvdWxkIGNyYXNo IGlmIHlvdSBoYXZlIGJ1ZyA3NTYwNDwvYm9keT4KPC9ib2R5PjwvaHRtbD5cclxuIiIiKTsKICAg ICAgICBlbHNlOgogICAgICAgICAgICBzLnNlbmRfcmVzcG9uc2UoNDIwLCAnYmFkIGNsaWVudCBu byBkb3VnaG51dCcpCiAgICAgICAgICAgIHMuc2VuZF9oZWFkZXIoJ0NhY2hlLUNvbnRyb2wnLCAn bm8tY2FjaGUnKQogICAgICAgICAgICBzLnNlbmRfaGVhZGVyKCdDb250ZW50LVR5cGUnLCAndGV4 dC9qYXZhc2NyaXB0JykKICAgICAgICAgICAgcy5lbmRfaGVhZGVycygpCiAgICAgICAgICAgICMg c2V0dGluZyBsaW5nZXIgd2l0aCBubyB0aW1lb3V0IG1ha2VzIGNsb3NlIHNlbmQgUlNUCiAgICAg ICAgICAgICMgcmF0aGVyIHRoYW4gRklOLgogICAgICAgICAgICBzLnJlcXVlc3Quc2V0c29ja29w dChzb2NrZXQuU09MX1NPQ0tFVCwgc29ja2V0LlNPX0xJTkdFUiwKICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgc3RydWN0LnBhY2soJ2lpJywgMSwgMCkpOwogICAgICAgICAgICBzLnJl cXVlc3Quc2VuZGFsbCgncGF5bG9hZCBkYXRhXHJcbicpOwogICAgICAgICAgICBzLnJlcXVlc3Qu Y2xvc2UoKTsKCmlmIF9fbmFtZV9fID09ICdfX21haW5fXyc6CiAgICBzZXJ2ZXJfY2xhc3MgPSBC YXNlSFRUUFNlcnZlci5IVFRQU2VydmVyCiAgICBodHRwZCA9IHNlcnZlcl9jbGFzcygoSE9TVF9O QU1FLCBQT1JUX05VTUJFUiksIE15SGFuZGxlcikKICAgIHByaW50IHRpbWUuYXNjdGltZSgpLCAi U2VydmVyIFN0YXJ0cyAtICVzOiVzIiAlIChIT1NUX05BTUUsIFBPUlRfTlVNQkVSKQogICAgdHJ5 OgogICAgICAgIGh0dHBkLnNlcnZlX2ZvcmV2ZXIoKQogICAgZXhjZXB0IEtleWJvYXJkSW50ZXJy dXB0OgogICAgICAgIHBhc3MKICAgIGh0dHBkLnNlcnZlcl9jbG9zZSgpCiAgICBwcmludCB0aW1l LmFzY3RpbWUoKSwgIlNlcnZlciBTdG9wcyAtICVzOiVzIiAlIChIT1NUX05BTUUsIFBPUlRfTlVN QkVSKQo= 113179 2011-11-01 09:44:48 -0700 2011-11-01 09:44:48 -0700 alternative reproduction lengthwrong.py text/x-python-script 1327 gavinp IyEvdXNyL2Jpbi9weXRob24KCmltcG9ydCB0aW1lCmltcG9ydCBzb2NrZXQKaW1wb3J0IEJhc2VI VFRQU2VydmVyCgoKSE9TVF9OQU1FID0gJzAuMC4wLjAnIApQT1JUX05VTUJFUiA9IDkwOTYKCmNs YXNzIE15SGFuZGxlcihCYXNlSFRUUFNlcnZlci5CYXNlSFRUUFJlcXVlc3RIYW5kbGVyKToKICAg IGRlZiBkb19HRVQocyk6CiAgICAgICAgIiIiUmVzcG9uZCB0byBhIEdFVCByZXF1ZXN0LiIiIgog ICAgICAgIGlmIHMucGF0aCA9PSAiLyI6CiAgICAgICAgICAgIHMuc2VuZF9yZXNwb25zZSgyMDAp OwogICAgICAgICAgICBzLmVuZF9oZWFkZXJzKCk7CiAgICAgICAgICAgIHMud2ZpbGUud3JpdGUo IiIiCjxodG1sPjxoZWFkPjxzY3JpcHQgYXN5bmMgc3JjPSJzY3JpcHQuanMiPjwvc2NyaXB0Pjwv aGVhZD4KPGJvZHk+dGVzdCBkb2N1bWVudCB0aGF0IHNob3VsZCBjcmFzaCBpZiB5b3UgaGF2ZSBi dWcgNzU2MDQ8L2JvZHk+CjwvYm9keT48L2h0bWw+IiIiKTsKICAgICAgICBlbHNlOgogICAgICAg ICAgICBzLnNlbmRfcmVzcG9uc2UoNTAwLCAnYmFkICcpCiAgICAgICAgICAgIHMuc2VuZF9oZWFk ZXIoJ0NvbnRlbnQtVHlwZScsICd0ZXh0L2phdmFzY3JpcHQnKQogICAgICAgICAgICBzLnNlbmRf aGVhZGVyKCdDb250ZW50LUxlbmd0aCcsICcxMDQ4NTc2JykKICAgICAgICAgICAgcy5zZW5kX2hl YWRlcignQ2FjaGUtQ29udHJvbCcsICduby1jYWNoZScpCiAgICAgICAgICAgIHMuZW5kX2hlYWRl cnMoKQogICAgICAgICAgICBtc2cgPSAnJwogICAgICAgICAgICBmb3IgaSBpbiByYW5nZSgxLDUp OgogICAgICAgICAgICAgICAgbXNnICs9ICdoaSBtb20gJWRcbicgJSBpOwogICAgICAgICAgICBz LnJlcXVlc3Quc2VuZGFsbChtc2cpCiAgICAgICAgICAgIHMucmVxdWVzdC5jbG9zZSgpCiAgICAg ICAgICAgICAgICAgICAgICAgIAoKaWYgX19uYW1lX18gPT0gJ19fbWFpbl9fJzoKICAgIHNlcnZl cl9jbGFzcyA9IEJhc2VIVFRQU2VydmVyLkhUVFBTZXJ2ZXIKICAgIGh0dHBkID0gc2VydmVyX2Ns YXNzKChIT1NUX05BTUUsIFBPUlRfTlVNQkVSKSwgTXlIYW5kbGVyKQogICAgcHJpbnQgdGltZS5h c2N0aW1lKCksICJTZXJ2ZXIgU3RhcnRzIC0gJXM6JXMiICUgKEhPU1RfTkFNRSwgUE9SVF9OVU1C RVIpCiAgICB0cnk6CiAgICAgICAgaHR0cGQuc2VydmVfZm9yZXZlcigpCiAgICBleGNlcHQgS2V5 Ym9hcmRJbnRlcnJ1cHQ6CiAgICAgICAgcGFzcwogICAgaHR0cGQuc2VydmVyX2Nsb3NlKCkKICAg IHByaW50IHRpbWUuYXNjdGltZSgpLCAiU2VydmVyIFN0b3BzIC0gJXM6JXMiICUgKEhPU1RfTkFN RSwgUE9SVF9OVU1CRVIpCg==