204524 2019-11-22 10:33:06 -0800 [iOS] Copy assertions before iterating over them in _notifyAssertionsOfImminentSuspension 2019-11-22 15:50:31 -0800 1 1 1 Unclassified WebKit WebKit2 WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 cdumez cdumez achristensen ap beidson ggaren webkit-bug-importer youennf oldest_to_newest 1592875 0 cdumez 2019-11-22 10:33:06 -0800 Copy assertions before iterating over them in _notifyAssertionsOfImminentSuspension. 1592876 1 cdumez 2019-11-22 10:33:23 -0800 <rdar://problem/57265830> 1592879 2 384166 cdumez 2019-11-22 10:35:08 -0800 Created attachment 384166 Patch 1593054 3 384166 ap 2019-11-22 15:16:38 -0800 Comment on attachment 384166 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384166&action=review > Source/WebKit/ChangeLog:13 > + them. It is common for process assertions to get released when uiAssertionWillExpireImminently() > + gets called, which would remove them from the _assertionsNeedingBackgroundTask vector we were > + iterating on. Is it possible to assertions to be added to the vector while iterating? 1593058 4 cdumez 2019-11-22 15:20:45 -0800 (In reply to Alexey Proskuryakov from comment #3) > Comment on attachment 384166 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=384166&action=review > > > Source/WebKit/ChangeLog:13 > > + them. It is common for process assertions to get released when uiAssertionWillExpireImminently() > > + gets called, which would remove them from the _assertionsNeedingBackgroundTask vector we were > > + iterating on. > > Is it possible to assertions to be added to the vector while iterating? Not as far as I know, we're telling the clients that their assertion has expired and they're supposed to abort whatever they're doing (and release their assertion). The "release their assertion" is logic I added recently, which is why this regressed. 1593069 5 384166 ap 2019-11-22 15:39:49 -0800 Comment on attachment 384166 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384166&action=review > Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm:115 > + Vector<WeakPtr<ProcessAndUIAssertion>> assertionsNeedingBackgroundTask = WTF::map(_assertionsNeedingBackgroundTask, [](auto* assertion) { Seems worth explaining in a comment why we don't expect anything to be added to _assertionsNeedingBackgroundTask while iterating - or maybe even ASSERT at the end that there isn't anything new. 1593075 6 cdumez 2019-11-22 15:50:31 -0800 Committed r252811: <https://trac.webkit.org/changeset/252811> 384166 2019-11-22 10:35:08 -0800 2019-11-22 15:39:49 -0800 Patch bug-204524-20191122103507.patch text/plain 2171 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjUyNzc1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDE0OTJjZWMyMjhhMzllZDNk ZmFhZTFjMjUxZDQ3N2E5YjA1ZDRiMDYuLmRhZTYzYzZkNjFmM2ZmNzgwOWEwY2RkNGYxZDJjNDk5 YTIwMzJkZDMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjAgQEAKKzIwMTktMTEtMjIgIENocmlzIER1 bWV6ICA8Y2R1bWV6QGFwcGxlLmNvbT4KKworICAgICAgICBbaU9TXSBDb3B5IGFzc2VydGlvbnMg YmVmb3JlIGl0ZXJhdGluZyBvdmVyIHRoZW0gaW4gX25vdGlmeUFzc2VydGlvbnNPZkltbWluZW50 U3VzcGVuc2lvbgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MjA0NTI0CisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS81NzI2NTgzMD4KKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBDb3B5IGFzc2VydGlvbnMgYmVm b3JlIGl0ZXJhdGluZyBvdmVyIHRoZW0gaW4gX25vdGlmeUFzc2VydGlvbnNPZkltbWluZW50U3Vz cGVuc2lvbiBhbmQgdXNlIFdlYWtQdHIKKyAgICAgICAgdG8gbWFrZSBzdXJlIHRoZSBhc3NlcnRp b25zIGFyZSBzdGlsbCBhbGl2ZSBiZWZvcmUgY2FsbGluZyB1aUFzc2VydGlvbldpbGxFeHBpcmVJ bW1pbmVudGx5KCkgb24KKyAgICAgICAgdGhlbS4gSXQgaXMgY29tbW9uIGZvciBwcm9jZXNzIGFz c2VydGlvbnMgdG8gZ2V0IHJlbGVhc2VkIHdoZW4gdWlBc3NlcnRpb25XaWxsRXhwaXJlSW1taW5l bnRseSgpCisgICAgICAgIGdldHMgY2FsbGVkLCB3aGljaCB3b3VsZCByZW1vdmUgdGhlbSBmcm9t IHRoZSBfYXNzZXJ0aW9uc05lZWRpbmdCYWNrZ3JvdW5kVGFzayB2ZWN0b3Igd2Ugd2VyZQorICAg ICAgICBpdGVyYXRpbmcgb24uCisKKyAgICAgICAgKiBVSVByb2Nlc3MvaW9zL1Byb2Nlc3NBc3Nl cnRpb25JT1MubW06CisgICAgICAgICgtW1dLUHJvY2Vzc0Fzc2VydGlvbkJhY2tncm91bmRUYXNr TWFuYWdlciBfbm90aWZ5QXNzZXJ0aW9uc09mSW1taW5lbnRTdXNwZW5zaW9uXSk6CisKIDIwMTkt MTEtMjIgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dhcmNpYUBpZ2FsaWEuY29tPgogCiAgICAg ICAgIFtHVEtdW1dQRV0gUmVtb3RlSW5zcGVjdG9yOiB1c2Ugc29ja2V0cyBpbnN0ZWFkIG9mIERC dXMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL2lvcy9Qcm9jZXNzQXNzZXJ0 aW9uSU9TLm1tIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvaW9zL1Byb2Nlc3NBc3NlcnRpb25J T1MubW0KaW5kZXggMzdhNjk4MmMzMGNmOGEyNjNjYmE5NWE0Y2Y1Y2ExZmJlNGNhMDEzNy4uM2Ni OGE3MjMxMWE5OTg2ZDE0NjRkNjI2YmY0Nzc0NTllZGQyYThhZSAxMDA2NDQKLS0tIGEvU291cmNl L1dlYktpdC9VSVByb2Nlc3MvaW9zL1Byb2Nlc3NBc3NlcnRpb25JT1MubW0KKysrIGIvU291cmNl L1dlYktpdC9VSVByb2Nlc3MvaW9zL1Byb2Nlc3NBc3NlcnRpb25JT1MubW0KQEAgLTExMiw4ICsx MTIsMTMgQEAgLSAodm9pZClfbm90aWZ5QXNzZXJ0aW9uc09mSW1taW5lbnRTdXNwZW5zaW9uCiB7 CiAgICAgQVNTRVJUKFJ1bkxvb3A6OmlzTWFpbigpKTsKIAotICAgIGZvciAoYXV0byogYXNzZXJ0 aW9uIDogY29weVRvVmVjdG9yKF9hc3NlcnRpb25zTmVlZGluZ0JhY2tncm91bmRUYXNrKSkKLSAg ICAgICAgYXNzZXJ0aW9uLT51aUFzc2VydGlvbldpbGxFeHBpcmVJbW1pbmVudGx5KCk7CisgICAg VmVjdG9yPFdlYWtQdHI8UHJvY2Vzc0FuZFVJQXNzZXJ0aW9uPj4gYXNzZXJ0aW9uc05lZWRpbmdC YWNrZ3JvdW5kVGFzayA9IFdURjo6bWFwKF9hc3NlcnRpb25zTmVlZGluZ0JhY2tncm91bmRUYXNr LCBbXShhdXRvKiBhc3NlcnRpb24pIHsKKyAgICAgICAgcmV0dXJuIG1ha2VXZWFrUHRyKCphc3Nl cnRpb24pOworICAgIH0pOworICAgIGZvciAoYXV0byBhc3NlcnRpb24gOiBhc3NlcnRpb25zTmVl ZGluZ0JhY2tncm91bmRUYXNrKSB7CisgICAgICAgIGlmIChhc3NlcnRpb24pCisgICAgICAgICAg ICBhc3NlcnRpb24tPnVpQXNzZXJ0aW9uV2lsbEV4cGlyZUltbWluZW50bHkoKTsKKyAgICB9CiB9 CiAKIAo=