180285 2017-12-01 14:57:23 -0800 WebSocketChannel should ensure its client is live when calling it in error case 2017-12-04 10:24:00 -0800 1 1 1 Unclassified WebKit WebCore Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 youennf youennf achristensen commit-queue darin ggaren webkit-bug-importer oldest_to_newest 1377084 0 youennf 2017-12-01 14:57:23 -0800 WebSocketChannel should ensure its client is live when calling it in error case 1377085 1 328164 youennf 2017-12-01 14:58:29 -0800 Created attachment 328164 Patch 1377086 2 youennf 2017-12-01 14:59:08 -0800 rdar://problem/32896043 1377510 3 328164 darin 2017-12-03 14:15:48 -0800 Comment on attachment 328164 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328164&action=review > Source/WebCore/ChangeLog:8 > + No observable change of behavior. That is strange. Can the pointer be null or not? 1377600 4 youennf 2017-12-03 21:20:33 -0800 (In reply to Darin Adler from comment #3) > Comment on attachment 328164 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=328164&action=review > > > Source/WebCore/ChangeLog:8 > > + No observable change of behavior. > > That is strange. Can the pointer be null or not? I believe it does. What I mean by no observable change of behavior is that this will not remove any error event trigger at JS level. 1377605 5 328164 commit-queue 2017-12-03 21:40:51 -0800 Comment on attachment 328164 Patch Clearing flags on attachment: 328164 Committed r225469: <https://trac.webkit.org/changeset/225469> 1377606 6 commit-queue 2017-12-03 21:40:53 -0800 All reviewed patches have been landed. Closing bug. 1377750 7 328164 ggaren 2017-12-04 10:21:00 -0800 Comment on attachment 328164 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328164&action=review >>> Source/WebCore/ChangeLog:8 >>> + No observable change of behavior. >> >> That is strange. Can the pointer be null or not? > > I believe it does. > What I mean by no observable change of behavior is that this will not remove any error event trigger at JS level. I think a clearer statement here is that we believe this fixes a common crash seen in diagnostic logging. (That is an observable change in behavior.) But we don't have a reproducible test case for the crash. The pointer can be null because disconnect() and didCloseSocketStream() set it to null. 1377751 8 ggaren 2017-12-04 10:21:32 -0800 > I think a clearer statement here is that we believe this fixes a common > crash seen in diagnostic logging. (That is an observable change in > behavior.) But we don't have a reproducible test case for the crash. The > pointer can be null because disconnect() and didCloseSocketStream() set it > to null. ... Youenn, have you tried to write a test case that invokes disconnect() or didCloseSocketStream()? 1377755 9 youennf 2017-12-04 10:24:00 -0800 (In reply to Geoffrey Garen from comment #8) > > I think a clearer statement here is that we believe this fixes a common > > crash seen in diagnostic logging. (That is an observable change in > > behavior.) But we don't have a reproducible test case for the crash. The > > pointer can be null because disconnect() and didCloseSocketStream() set it > > to null. > > ... Youenn, have you tried to write a test case that invokes disconnect() or > didCloseSocketStream()? I haven't tried but will do. 328164 2017-12-01 14:58:29 -0800 2017-12-03 21:40:51 -0800 Patch bug-180285-20171201145828.patch text/plain 1652 youennf U3VidmVyc2lvbiBSZXZpc2lvbjogMjI1NDEyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjJjNjY2YTI0ZWRhNWM1 ZjZmN2YyZDBjZjQwNDFmMTQ1MWIxOGY3Ni4uZjhkYWIzZjgxOTUxYjFiMjllNjFiMGVkZGE3YzQ4 N2M2ZTg1NGQwNyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE4IEBACiAyMDE3LTEyLTAxICBZb3Vl bm4gRmFibGV0ICA8eW91ZW5uQGFwcGxlLmNvbT4KIAorICAgICAgICBXZWJTb2NrZXRDaGFubmVs IHNob3VsZCBlbnN1cmUgaXRzIGNsaWVudCBpcyBsaXZlIHdoZW4gY2FsbGluZyBpdCBpbiBlcnJv ciBjYXNlCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0x ODAyODUKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBO byBvYnNlcnZhYmxlIGNoYW5nZSBvZiBiZWhhdmlvci4KKyAgICAgICAgVGhpcyBtYWtlcyBpdCBj b25zaXN0ZW50IHdpdGggb3RoZXIgY2FsbHMgb2YgZGlkUmVjZWl2ZU1lc3NhZ2VFcnJvci4KKwor ICAgICAgICAqIE1vZHVsZXMvd2Vic29ja2V0cy9XZWJTb2NrZXRDaGFubmVsLmNwcDoKKyAgICAg ICAgKFdlYkNvcmU6OldlYlNvY2tldENoYW5uZWw6OmZhaWwpOgorCisyMDE3LTEyLTAxICBZb3Vl bm4gRmFibGV0ICA8eW91ZW5uQGFwcGxlLmNvbT4KKwogICAgICAgICBJbXBsZW1lbnQgaHR0cHM6 Ly93M2MuZ2l0aHViLmlvL1NlcnZpY2VXb3JrZXIvI2NsaWVudHMtZ2V0CiAgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xODAxNjcKIApkaWZmIC0tZ2l0IGEv U291cmNlL1dlYkNvcmUvTW9kdWxlcy93ZWJzb2NrZXRzL1dlYlNvY2tldENoYW5uZWwuY3BwIGIv U291cmNlL1dlYkNvcmUvTW9kdWxlcy93ZWJzb2NrZXRzL1dlYlNvY2tldENoYW5uZWwuY3BwCmlu ZGV4IDM5ZjMxMDA3Zjg0OTVlYjEwZTM1OWNlYzJmODFjNGE2M2ZlYWY4NGYuLmE1OWI0NjllM2M5 YzVmNWM5OTY0NWY5MGQyNGY2OGUzY2E5MzFkZGMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3Jl L01vZHVsZXMvd2Vic29ja2V0cy9XZWJTb2NrZXRDaGFubmVsLmNwcAorKysgYi9Tb3VyY2UvV2Vi Q29yZS9Nb2R1bGVzL3dlYnNvY2tldHMvV2ViU29ja2V0Q2hhbm5lbC5jcHAKQEAgLTIzMiw3ICsy MzIsOCBAQCB2b2lkIFdlYlNvY2tldENoYW5uZWw6OmZhaWwoY29uc3QgU3RyaW5nJiByZWFzb24p CiAgICAgbV9kZWZsYXRlRnJhbWVyLmRpZEZhaWwoKTsKICAgICBtX2hhc0NvbnRpbnVvdXNGcmFt ZSA9IGZhbHNlOwogICAgIG1fY29udGludW91c0ZyYW1lRGF0YS5jbGVhcigpOwotICAgIG1fY2xp ZW50LT5kaWRSZWNlaXZlTWVzc2FnZUVycm9yKCk7CisgICAgaWYgKG1fY2xpZW50KQorICAgICAg ICBtX2NsaWVudC0+ZGlkUmVjZWl2ZU1lc3NhZ2VFcnJvcigpOwogCiAgICAgaWYgKG1faGFuZGxl ICYmICFtX2Nsb3NlZCkKICAgICAgICAgbV9oYW5kbGUtPmRpc2Nvbm5lY3QoKTsgLy8gV2lsbCBj YWxsIGRpZENsb3NlU29ja2V0U3RyZWFtKCkgYnV0IG1heWJlIG5vdCBzeW5jaHJvbm91c2x5Lgo=