RESOLVED FIXED 204524
[iOS] Copy assertions before iterating over them in _notifyAssertionsOfImminentSuspension
https://bugs.webkit.org/show_bug.cgi?id=204524
Summary [iOS] Copy assertions before iterating over them in _notifyAssertionsOfImmine...
Chris Dumez
Reported 2019-11-22 10:33:06 PST
Copy assertions before iterating over them in _notifyAssertionsOfImminentSuspension.
Attachments
Patch (2.12 KB, patch)
2019-11-22 10:35 PST, Chris Dumez
ap: review+
Chris Dumez
Comment 1 2019-11-22 10:33:23 PST
Chris Dumez
Comment 2 2019-11-22 10:35:08 PST
Alexey Proskuryakov
Comment 3 2019-11-22 15:16:38 PST
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?
Chris Dumez
Comment 4 2019-11-22 15:20:45 PST
(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.
Alexey Proskuryakov
Comment 5 2019-11-22 15:39:49 PST
Comment on attachment 384166 [details] 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.
Chris Dumez
Comment 6 2019-11-22 15:50:31 PST
Note You need to log in before you can comment on or make changes to this bug.