107050 2013-01-16 13:34:32 -0800 IndexedDB: Possible null ScriptExecutionContext passed to callbacks during frame destruction 2013-01-17 11:28:01 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED INVALID P2 Normal --- 36565 107141 107146 1 jsbell jsbell alecflett dgrogan tony webkit.review.bot oldest_to_newest 808663 0 jsbell 2013-01-16 13:34:32 -0800 Trying to track down a null-dereference crash involving Workers and IndexedDB, one possibility is that the generated binding code gets a null context from WorkerScriptController::controllerForContext() and passes this into DOM callbacks (including IDB). It seems unlikely, but we'd like to confirm or refute the hypothesis. 808669 1 183029 jsbell 2013-01-16 13:38:08 -0800 Created attachment 183029 Patch 808702 2 183029 tony 2013-01-16 14:09:24 -0800 Comment on attachment 183029 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183029&action=review > Source/WebCore/ChangeLog:7 > + Temporary code to defend against null contexts. Will either refute a hypothesis, or we'll > + need to make a more systemic fix elsewhere. Either way it will be removed in a few days. How will we decide whether it worked or not? Is this something we're seeing in the wild or something that cluster fuzz caught? I'm a bit nervous about this even if it fixes the crash because it might be hiding some other bug. We need a repro case to be certain. Someone who knows more about frame destruction (abarth?) can probably comment whether this ought to be needed or not. 808711 3 jsbell 2013-01-16 14:20:22 -0800 Sorry, should have linked to crbug.com/168503 for context. We've been unable to repro locally, but some users can do so reliably with a simple repro case. We're still working on trying to repro it ourselves using virtual machines, etc. 808721 4 tony 2013-01-16 14:27:50 -0800 I see, you might want to mention that WorkerScriptController::controllerForContext() does a similar check. Does the change that added the check in WorkerScriptController::controllerForContext() include a test for this case? Maybe that will provide some hints? I would also mention in the changelog that we're unable to repro and we're watching crash reports from users. 808722 5 183029 tony 2013-01-16 14:28:40 -0800 Comment on attachment 183029 Patch Seems OK to try, but please explain more in the ChangeLog. 808735 6 jsbell 2013-01-16 14:43:18 -0800 Thanks, tony@ - great ideas. For reference: https://bugs.webkit.org/show_bug.cgi?id=36565 - doesn't look like new tests were added. 808738 7 tony 2013-01-16 14:50:49 -0800 (In reply to comment #6) > Thanks, tony@ - great ideas. > > For reference: > > https://bugs.webkit.org/show_bug.cgi?id=36565 - doesn't look like new tests were added. That was a code move. The original patch that added the lines is https://bugs.webkit.org/show_bug.cgi?id=28004 , which also doesn't have a test :( 808746 8 183039 jsbell 2013-01-16 14:55:42 -0800 Created attachment 183039 Patch for landing 808756 9 183039 webkit.review.bot 2013-01-16 15:12:32 -0800 Comment on attachment 183039 Patch for landing Clearing flags on attachment: 183039 Committed r139929: <http://trac.webkit.org/changeset/139929> 808757 10 webkit.review.bot 2013-01-16 15:12:36 -0800 All reviewed patches have been landed. Closing bug. 809431 11 jsbell 2013-01-17 10:10:52 -0800 Reports from users that the crash still occurs. After verifying, will likely back this out. 809444 12 webkit.review.bot 2013-01-17 10:26:34 -0800 Re-opened since this is blocked by bug 107141 809448 13 jsbell 2013-01-17 10:30:40 -0800 Rolled this out. Closing as INVALID since this wasn't the proximal cause of the crashes we're seeing. 183029 2013-01-16 13:38:08 -0800 2013-01-16 14:55:40 -0800 Patch bug-107050-20130116133509.patch text/plain 1938 jsbell U3VidmVyc2lvbiBSZXZpc2lvbjogMTM5ODM4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNTE1ZDllZjQyOThmZjgw MWY5NzA3NWQ4YTgzMjMzODNmY2NjZDA5YS4uNDRlZWEzMzM3MGFhZGJmNGU2OWQyMzk5NTc5NmQ2 ZDJkNDUzZjYyNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDEzLTAxLTE2ICBKb3No dWEgQmVsbCAgPGpzYmVsbEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgSW5kZXhlZERCOiBQb3Nz aWJsZSBudWxsIFNjcmlwdEV4ZWN1dGlvbkNvbnRleHQgcGFzc2VkIHRvIGNhbGxiYWNrcyBkdXJp bmcgZnJhbWUgZGVzdHJ1Y3Rpb24KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTEwNzA1MAorCisgICAgICAgIFRlbXBvcmFyeSBjb2RlIHRvIGRlZmVuZCBh Z2FpbnN0IG51bGwgY29udGV4dHMuIFdpbGwgZWl0aGVyIHJlZnV0ZSBhIGh5cG90aGVzaXMsIG9y IHdlJ2xsCisgICAgICAgIG5lZWQgdG8gbWFrZSBhIG1vcmUgc3lzdGVtaWMgZml4IGVsc2V3aGVy ZS4gRWl0aGVyIHdheSBpdCB3aWxsIGJlIHJlbW92ZWQgaW4gYSBmZXcgZGF5cy4KKworICAgICAg ICBObyBuZXcgdGVzdHMgLSB0aGlzIHNob3VsZG4ndCBiZSBoYXBwZW5pbmcuCisKKyAgICAgICAg KiBNb2R1bGVzL2luZGV4ZWRkYi9JREJPYmplY3RTdG9yZS5jcHA6CisgICAgICAgIChXZWJDb3Jl OjpJREJPYmplY3RTdG9yZTo6Y3JlYXRlSW5kZXgpOgorCiAyMDEzLTAxLTE1ICBSeW9zdWtlIE5p d2EgIDxybml3YUB3ZWJraXQub3JnPgogCiAgICAgICAgIEdUSysgYW5kIFF0IGJ1aWxkIGZpeCBh ZnRlciByMTM5ODMzLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvTW9kdWxlcy9pbmRleGVk ZGIvSURCT2JqZWN0U3RvcmUuY3BwIGIvU291cmNlL1dlYkNvcmUvTW9kdWxlcy9pbmRleGVkZGIv SURCT2JqZWN0U3RvcmUuY3BwCmluZGV4IGYxMGQwODZlZTQ5NmZkZTFiNTgxODA4ZTk5YzlhZTdl YTk0NWMzNDQuLmY4ODkyZGVhZGE1NjViZjdmZmQxYTA0NTE2N2U0ZjBmYzI5YzNjZWYgMTAwNjQ0 Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvaW5kZXhlZGRiL0lEQk9iamVjdFN0b3JlLmNw cAorKysgYi9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL2luZGV4ZWRkYi9JREJPYmplY3RTdG9yZS5j cHAKQEAgLTM1Niw2ICszNTYsMTIgQEAgUGFzc1JlZlB0cjxJREJJbmRleD4gSURCT2JqZWN0U3Rv cmU6OmNyZWF0ZUluZGV4KFNjcmlwdEV4ZWN1dGlvbkNvbnRleHQqIGNvbnRleHQKIFBhc3NSZWZQ dHI8SURCSW5kZXg+IElEQk9iamVjdFN0b3JlOjpjcmVhdGVJbmRleChTY3JpcHRFeGVjdXRpb25D b250ZXh0KiBjb250ZXh0LCBjb25zdCBTdHJpbmcmIG5hbWUsIGNvbnN0IElEQktleVBhdGgmIGtl eVBhdGgsIGJvb2wgdW5pcXVlLCBib29sIG11bHRpRW50cnksIEV4Y2VwdGlvbkNvZGUmIGVjKQog ewogICAgIElEQl9UUkFDRSgiSURCT2JqZWN0U3RvcmU6OmNyZWF0ZUluZGV4Iik7CisgICAgLy8g RklYTUU6IFRlbXBvcmFyeSBjb2RlIHRvIGRldGVybWluZSBpZiBudWxsIGNvbnRleHRzIGFyZSBw bGF1c2libGUgZHVyaW5nIGZyYW1lIGRlc3RydWN0aW9uLgorICAgIC8vIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMDcwNTAKKyAgICBBU1NFUlQoY29udGV4dCk7Cisg ICAgaWYgKCFjb250ZXh0KQorICAgICAgICByZXR1cm4gMDsKKwogICAgIGlmICghbV90cmFuc2Fj dGlvbi0+aXNWZXJzaW9uQ2hhbmdlKCkgfHwgbV9kZWxldGVkKSB7CiAgICAgICAgIGVjID0gSURC RGF0YWJhc2VFeGNlcHRpb246OkludmFsaWRTdGF0ZUVycm9yOwogICAgICAgICByZXR1cm4gMDsK 183039 2013-01-16 14:55:42 -0800 2013-01-16 15:12:32 -0800 Patch for landing bug-107050-20130116145243.patch text/plain 2235 jsbell U3VidmVyc2lvbiBSZXZpc2lvbjogMTM5ODM4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNTE1ZDllZjQyOThmZjgw MWY5NzA3NWQ4YTgzMjMzODNmY2NjZDA5YS4uMzJlNWExNzc3YzE1ZmEwMWVlYzAyNmNiYzBmNmJi YmFmYzAxODkwMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIxIEBACisyMDEzLTAxLTE2ICBKb3No dWEgQmVsbCAgPGpzYmVsbEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgSW5kZXhlZERCOiBQb3Nz aWJsZSBudWxsIFNjcmlwdEV4ZWN1dGlvbkNvbnRleHQgcGFzc2VkIHRvIGNhbGxiYWNrcyBkdXJp bmcgZnJhbWUgZGVzdHJ1Y3Rpb24KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTEwNzA1MAorCisgICAgICAgIFJldmlld2VkIGJ5IFRvbnkgQ2hhbmcuCisK KyAgICAgICAgVGVtcG9yYXJ5IGNvZGUgdG8gZGVmZW5kIGFnYWluc3QgbnVsbCBjb250ZXh0cy4g V2lsbCBlaXRoZXIgcmVmdXRlIGEgaHlwb3RoZXNpcywgb3Igd2UnbGwKKyAgICAgICAgbmVlZCB0 byBtYWtlIGEgbW9yZSBzeXN0ZW1pYyBmaXggZWxzZXdoZXJlLiBFaXRoZXIgd2F5IGl0IHdpbGwg YmUgcmVtb3ZlZCBpbiBhIGZldyBkYXlzLgorICAgICAgICBXZSdyZSB1bmFibGUgdG8gcmVwcm8s IGJ1dCB3YXRjaGluZyBjcmFzaCByZXBvcnRzIGZyb20gdXNlcnMuIE9uZSBwb3NzaWJsZSBzb3Vy Y2UgaXMKKyAgICAgICAgdGhhdCBkdXJpbmcgcGFnZSB0ZWFyLWRvd24gV29ya2VyU2NyaXB0Q29u dHJvbGxlcjo6Y29udHJvbGxlckZvckNvbnRleHQoKSByZXR1cm5zIG51bGwKKyAgICAgICAgKHRo ZXJlJ3MgYSBjb21tZW50IGFib3V0IHRoYXQgY2FzZSkgbGVhZGluZyB0byBhIG51bGwgY29udGV4 dC4KKworICAgICAgICBObyBuZXcgdGVzdHMgLSB0aGlzIHNob3VsZG4ndCBiZSBoYXBwZW5pbmcu CisKKyAgICAgICAgKiBNb2R1bGVzL2luZGV4ZWRkYi9JREJPYmplY3RTdG9yZS5jcHA6CisgICAg ICAgIChXZWJDb3JlOjpJREJPYmplY3RTdG9yZTo6Y3JlYXRlSW5kZXgpOgorCiAyMDEzLTAxLTE1 ICBSeW9zdWtlIE5pd2EgIDxybml3YUB3ZWJraXQub3JnPgogCiAgICAgICAgIEdUSysgYW5kIFF0 IGJ1aWxkIGZpeCBhZnRlciByMTM5ODMzLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvTW9k dWxlcy9pbmRleGVkZGIvSURCT2JqZWN0U3RvcmUuY3BwIGIvU291cmNlL1dlYkNvcmUvTW9kdWxl cy9pbmRleGVkZGIvSURCT2JqZWN0U3RvcmUuY3BwCmluZGV4IGYxMGQwODZlZTQ5NmZkZTFiNTgx ODA4ZTk5YzlhZTdlYTk0NWMzNDQuLmY4ODkyZGVhZGE1NjViZjdmZmQxYTA0NTE2N2U0ZjBmYzI5 YzNjZWYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvaW5kZXhlZGRiL0lEQk9i amVjdFN0b3JlLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL2luZGV4ZWRkYi9JREJP YmplY3RTdG9yZS5jcHAKQEAgLTM1Niw2ICszNTYsMTIgQEAgUGFzc1JlZlB0cjxJREJJbmRleD4g SURCT2JqZWN0U3RvcmU6OmNyZWF0ZUluZGV4KFNjcmlwdEV4ZWN1dGlvbkNvbnRleHQqIGNvbnRl eHQKIFBhc3NSZWZQdHI8SURCSW5kZXg+IElEQk9iamVjdFN0b3JlOjpjcmVhdGVJbmRleChTY3Jp cHRFeGVjdXRpb25Db250ZXh0KiBjb250ZXh0LCBjb25zdCBTdHJpbmcmIG5hbWUsIGNvbnN0IElE QktleVBhdGgmIGtleVBhdGgsIGJvb2wgdW5pcXVlLCBib29sIG11bHRpRW50cnksIEV4Y2VwdGlv bkNvZGUmIGVjKQogewogICAgIElEQl9UUkFDRSgiSURCT2JqZWN0U3RvcmU6OmNyZWF0ZUluZGV4 Iik7CisgICAgLy8gRklYTUU6IFRlbXBvcmFyeSBjb2RlIHRvIGRldGVybWluZSBpZiBudWxsIGNv bnRleHRzIGFyZSBwbGF1c2libGUgZHVyaW5nIGZyYW1lIGRlc3RydWN0aW9uLgorICAgIC8vIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMDcwNTAKKyAgICBBU1NFUlQo Y29udGV4dCk7CisgICAgaWYgKCFjb250ZXh0KQorICAgICAgICByZXR1cm4gMDsKKwogICAgIGlm ICghbV90cmFuc2FjdGlvbi0+aXNWZXJzaW9uQ2hhbmdlKCkgfHwgbV9kZWxldGVkKSB7CiAgICAg ICAgIGVjID0gSURCRGF0YWJhc2VFeGNlcHRpb246OkludmFsaWRTdGF0ZUVycm9yOwogICAgICAg ICByZXR1cm4gMDsK