125110 2013-12-02 15:12:10 -0800 Possible crash in void ProgressTracker::progressHeartbeatTimerFired(Timer<ProgressTracker>*) 2014-01-19 21:00:08 -0800 1 1 1 Unclassified WebKit Page Loading 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 beidson beidson commit-queue ddkilzer japhet oldest_to_newest 955620 0 beidson 2013-12-02 15:12:10 -0800 Possible crash in void ProgressTracker::progressHeartbeatTimerFired(Timer<ProgressTracker>*) We know that it is possible to crash in ProgressTracker::progressHeartbeatTimerFired(Timer<ProgressTracker>*) with a null m_originatingProgressFrame. On the surface it appears this should not be possible because any time m_originatingProgressFrame is cleared out the progress heartbeat timer is also stopped. It is likely a race condition with stopping the timer in multi-threaded environments, yet we still have no way to reproduce. There's no harm in null checking m_originatingProgressFrame before calling in to its loader. 955628 1 218223 beidson 2013-12-02 15:17:17 -0800 Created attachment 218223 Patch v1 955631 2 218223 darin 2013-12-02 15:22:42 -0800 Comment on attachment 218223 Patch v1 While there is no harm in adding a null check, if this is actually a race condition, then a null check is likely not sufficient to fix the problem. It might, however, reduce the frequency of a crash. 955640 3 beidson 2013-12-02 15:28:57 -0800 (In reply to comment #2) > (From update of attachment 218223 [details]) > While there is no harm in adding a null check, if this is actually a race condition, then a null check is likely not sufficient to fix the problem. It might, however, reduce the frequency of a crash. Agreed. Based on looking at disassembly this is our only theory on what's going on, so a more comprehensive fix isn't presenting itself yet. Landed in http://trac.webkit.org/changeset/159974 955681 4 beidson 2013-12-02 16:39:56 -0800 This null check might have been incorrect 955683 5 beidson 2013-12-02 16:40:51 -0800 More specifically, ProgressTracker::progressHeartbeatTimerFired(Timer<ProgressTracker>*) might be calling in to a valid m_originatingProgressFrame (and its valid loader), but in FrameLoader::loadProgressingStatusChanged() we might have a null FrameView. 955684 6 218237 beidson 2013-12-02 16:43:09 -0800 Created attachment 218237 Patch v2 - Also null check a FrameView 955685 7 218238 beidson 2013-12-02 16:43:09 -0800 Created attachment 218238 955707 8 beidson 2013-12-02 17:24:37 -0800 http://trac.webkit.org/changeset/159986 969684 9 ddkilzer 2014-01-19 21:00:08 -0800 <rdar://problem/15217862> 218223 2013-12-02 15:17:17 -0800 2013-12-02 15:22:42 -0800 Patch v1 patch text/plain 1798 beidson ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA4NWU5OTJkLi5mMzRlYjU0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMjQg QEAKIDIwMTMtMTItMDIgIEJyYWR5IEVpZHNvbiAgPGJlaWRzb25AYXBwbGUuY29tPgogCisgICAg ICAgIFBvc3NpYmxlIGNyYXNoIGluIFByb2dyZXNzVHJhY2tlcjo6cHJvZ3Jlc3NIZWFydGJlYXRU aW1lckZpcmVkKFRpbWVyPFByb2dyZXNzVHJhY2tlcj4qKQorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTI1MTEwCisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSXTigJlzIHBvc3NpYmxlIHRvIGhhdmUgYSBudWxs IG1fb3JpZ2luYXRpbmdQcm9ncmVzc0ZyYW1lIHdoZW4gdGhlIGhlYXJ0YmVhdCB0aW1lciBmaXJl cy4KKworICAgICAgICBPbiB0aGUgc3VyZmFjZSB0aGlzIHNlZW1zIGltcG9zc2libGUgYmVjYXVz ZSB0aGUgb25seSB0aW1lIG1fb3JpZ2luYXRpbmdQcm9ncmVzc0ZyYW1lIGlzIGNsZWFyZWQKKyAg ICAgICAgb3V0IHRoZSBoZWFydGJlYXQgdGltZXIgaXMgYWxzbyBzdG9wcGVkLgorCisgICAgICAg IEJ1dCB0aGVyZeKAmXMgbGlrZWx5IHN0aWxsIGEgcmFjZSBjb25kaXRpb24gaW4gbXVsdGktdGhy ZWFkZWQgZW52aXJvbm1lbnRzLgorCisgICAgICAgIFRoZXJl4oCZcyBubyBoYXJtIGluIG51bGwt Y2hlY2tpbmcgbV9vcmlnaW5hdGluZ1Byb2dyZXNzRnJhbWUgYmVmb3JlIGFjY2Vzc2luZyBpdHMg bG9hZGVyLgorCisgICAgICAgICogbG9hZGVyL1Byb2dyZXNzVHJhY2tlci5jcHA6CisgICAgICAg IChXZWJDb3JlOjpQcm9ncmVzc1RyYWNrZXI6OnByb2dyZXNzSGVhcnRiZWF0VGltZXJGaXJlZCk6 CisKKzIwMTMtMTItMDIgIEJyYWR5IEVpZHNvbiAgPGJlaWRzb25AYXBwbGUuY29tPgorCiAgICAg ICAgIEFkZCBtb3JlIENhY2hlZFBhZ2UgbnVsbCBjaGVja3MKICAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEyNTEwNgogCmRpZmYgLS1naXQgYS9Tb3VyY2Uv V2ViQ29yZS9sb2FkZXIvUHJvZ3Jlc3NUcmFja2VyLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2xvYWRl ci9Qcm9ncmVzc1RyYWNrZXIuY3BwCmluZGV4IDZjOWIzNWMuLmMxMTAzZTAgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJDb3JlL2xvYWRlci9Qcm9ncmVzc1RyYWNrZXIuY3BwCisrKyBiL1NvdXJjZS9X ZWJDb3JlL2xvYWRlci9Qcm9ncmVzc1RyYWNrZXIuY3BwCkBAIC0yOTUsNyArMjk1LDggQEAgdm9p ZCBQcm9ncmVzc1RyYWNrZXI6OnByb2dyZXNzSGVhcnRiZWF0VGltZXJGaXJlZChUaW1lcjxQcm9n cmVzc1RyYWNrZXI+KikKIAogICAgIG1fdG90YWxCeXRlc1JlY2VpdmVkQmVmb3JlUHJldmlvdXNI ZWFydGJlYXQgPSBtX3RvdGFsQnl0ZXNSZWNlaXZlZDsKIAotICAgIG1fb3JpZ2luYXRpbmdQcm9n cmVzc0ZyYW1lLT5sb2FkZXIoKS5sb2FkUHJvZ3Jlc3NpbmdTdGF0dXNDaGFuZ2VkKCk7CisgICAg aWYgKG1fb3JpZ2luYXRpbmdQcm9ncmVzc0ZyYW1lKQorICAgICAgICBtX29yaWdpbmF0aW5nUHJv Z3Jlc3NGcmFtZS0+bG9hZGVyKCkubG9hZFByb2dyZXNzaW5nU3RhdHVzQ2hhbmdlZCgpOwogCiAg ICAgaWYgKG1fcHJvZ3Jlc3NWYWx1ZSA+PSBmaW5hbFByb2dyZXNzVmFsdWUpCiAgICAgICAgIG1f cHJvZ3Jlc3NIZWFydGJlYXRUaW1lci5zdG9wKCk7Cg== 218237 2013-12-02 16:43:09 -0800 2013-12-02 17:17:48 -0800 Patch v2 - Also null check a FrameView patch text/plain 1523 beidson ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBiZWNhNjFkLi5iNDZiZGFhIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0zLDYgKzMsMjAg QEAKICAgICAgICAgUG9zc2libGUgY3Jhc2ggaW4gUHJvZ3Jlc3NUcmFja2VyOjpwcm9ncmVzc0hl YXJ0YmVhdFRpbWVyRmlyZWQoVGltZXI8UHJvZ3Jlc3NUcmFja2VyPiopCiAgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMjUxMTAKIAorICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBGcmFtZUxvYWRlcjo6bG9hZFByb2dy ZXNzaW5nU3RhdHVzQ2hhbmdlZCgpIG1pZ2h0IGJlIGNhbGxlZCB3aGlsZSB0aGUgRnJhbWUgaGFz IGEgbnVsbCBGcmFtZVZpZXcuCisKKyAgICAgICAgSXTigJlzIHVuY2xlYXIgaG93IHRvIHJlcHJv ZHVjZSwgYnV0IHRoZXJl4oCZcyBubyBoYXJtIGluIGEgbnVsbCBjaGVjay4KKworICAgICAgICAq IGxvYWRlci9GcmFtZUxvYWRlci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpGcmFtZUxvYWRlcjo6 bG9hZFByb2dyZXNzaW5nU3RhdHVzQ2hhbmdlZCk6CisKKzIwMTMtMTItMDIgIEJyYWR5IEVpZHNv biAgPGJlaWRzb25AYXBwbGUuY29tPgorCisgICAgICAgIFBvc3NpYmxlIGNyYXNoIGluIFByb2dy ZXNzVHJhY2tlcjo6cHJvZ3Jlc3NIZWFydGJlYXRUaW1lckZpcmVkKFRpbWVyPFByb2dyZXNzVHJh Y2tlcj4qKQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTI1MTEwCisKICAgICAgICAgUmV2aWV3ZWQgYnkgRGFyaW4gQWRsZXIuCiAKICAgICAgICAgSXTi gJlzIHBvc3NpYmxlIHRvIGhhdmUgYSBudWxsIG1fb3JpZ2luYXRpbmdQcm9ncmVzc0ZyYW1lIHdo ZW4gdGhlIGhlYXJ0YmVhdCB0aW1lciBmaXJlcy4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3Jl L2xvYWRlci9GcmFtZUxvYWRlci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvRnJhbWVMb2Fk ZXIuY3BwCmluZGV4IGYzNDc0YzMuLmFmZTY2M2UgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3Jl L2xvYWRlci9GcmFtZUxvYWRlci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvbG9hZGVyL0ZyYW1l TG9hZGVyLmNwcApAQCAtMzM4MCw2ICszMzgwLDkgQEAgTmV0d29ya2luZ0NvbnRleHQqIEZyYW1l TG9hZGVyOjpuZXR3b3JraW5nQ29udGV4dCgpIGNvbnN0CiB2b2lkIEZyYW1lTG9hZGVyOjpsb2Fk UHJvZ3Jlc3NpbmdTdGF0dXNDaGFuZ2VkKCkKIHsKICAgICBGcmFtZVZpZXcqIHZpZXcgPSBtX2Zy YW1lLm1haW5GcmFtZSgpLnZpZXcoKTsKKyAgICBpZiAoIXZpZXcpCisgICAgICAgIHJldHVybjsK KwogICAgIHZpZXctPnVwZGF0ZUxheWVyRmx1c2hUaHJvdHRsaW5nSW5BbGxGcmFtZXMoKTsKICAg ICB2aWV3LT5hZGp1c3RUaWxlZEJhY2tpbmdDb3ZlcmFnZSgpOwogfQo=