Skip to content

Commit

Permalink
Crash under ReportingScope::unregisterReportingObserver()
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=260038
rdar://113533957

Reviewed by David Kilzer.

The ReportingScope keeps the ReportingObservers alive via its `m_reportingObservers`
vector. The crash would happen because ReportingScope::removeAllObservers() would
call clear() on this vector, which may cause ReportingObserver objects to get
destroyed. In turn, the ReportingObserver destructor would call
ReportingScope::unregisterReportingObserver() to unregister itself. This would
try to modify the vector while it is in the middle of getting cleared.

To address the issue, the ReportingObserver destructor no longer attempts to
unregister itself from the ReportingScope. Since the ReportingScope keeps a
strong reference to the observers, there is no way the observer is still
registered if its destructor gets called.

* Source/WebCore/Modules/reporting/ReportingObserver.cpp:
(WebCore::ReportingObserver::~ReportingObserver): Deleted.
* Source/WebCore/Modules/reporting/ReportingScope.cpp:
(WebCore::ReportingScope::unregisterReportingObserver):

Canonical link: https://commits.webkit.org/266791@main
  • Loading branch information
cdumez committed Aug 10, 2023
1 parent 75a3eb3 commit a7dc74b
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 5 deletions.
5 changes: 1 addition & 4 deletions Source/WebCore/Modules/reporting/ReportingObserver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,7 @@ ReportingObserver::ReportingObserver(ScriptExecutionContext& scriptExecutionCont
{
}

ReportingObserver::~ReportingObserver()
{
disconnect();
}
ReportingObserver::~ReportingObserver() = default;

void ReportingObserver::disconnect()
{
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/Modules/reporting/ReportingScope.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ void ReportingScope::registerReportingObserver(ReportingObserver& observer)

void ReportingScope::unregisterReportingObserver(ReportingObserver& observer)
{
m_reportingObservers.removeFirstMatching([&observer](auto item) {
m_reportingObservers.removeFirstMatching([&observer](auto& item) {
return item.ptr() == &observer;
});
}
Expand Down

0 comments on commit a7dc74b

Please sign in to comment.