Commit 4c572223 authored by Raymond Toy's avatar Raymond Toy Committed by Commit Bot

Make finished_source_handlers_ hold scoped_refptrs

Previously, finished_source_handlers_ held raw pointers to
AudioHandlers and assumed that active_source_handlers_ also had a
copy.  But when the context goes away, active_source_handlers_ would
be cleared, but not finished_source_handlers_, leaving pointers to
deleted objects.

So do two things:
1. Change finished_source_handlers_ to hold scoped_refptrs to manage
   lifetime of the objects
2. Clear finished_source_handler_ in ClearHandlersToBeDeleted()

Either of these fix the repro case, but let's do both.  Don't want to
leaving dangling objects.

Manually tested the repro case which no longer reproduces.

Bug: 1059686
Change-Id: I2f30c996e8589fa5c3890d32500c4bb4f3bc4286
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2098260Reviewed-by: default avatarHongchan Choi <hongchan@chromium.org>
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749302}
parent c02a83ec
...@@ -77,9 +77,7 @@ void DeferredTaskHandler::BreakConnections() { ...@@ -77,9 +77,7 @@ void DeferredTaskHandler::BreakConnections() {
// connection. // connection.
wtf_size_t size = finished_source_handlers_.size(); wtf_size_t size = finished_source_handlers_.size();
if (size > 0) { if (size > 0) {
for (auto* finished : finished_source_handlers_) { for (auto finished : finished_source_handlers_) {
// Break connection first and then remove from the list because that can
// cause the handler to be deleted.
finished->BreakConnectionWithLock(); finished->BreakConnectionWithLock();
active_source_handlers_.erase(finished); active_source_handlers_.erase(finished);
} }
...@@ -358,6 +356,7 @@ void DeferredTaskHandler::ClearHandlersToBeDeleted() { ...@@ -358,6 +356,7 @@ void DeferredTaskHandler::ClearHandlersToBeDeleted() {
deletable_orphan_handlers_.clear(); deletable_orphan_handlers_.clear();
automatic_pull_handlers_.clear(); automatic_pull_handlers_.clear();
rendering_automatic_pull_handlers_.clear(); rendering_automatic_pull_handlers_.clear();
finished_source_handlers_.clear();
active_source_handlers_.clear(); active_source_handlers_.clear();
} }
......
...@@ -190,7 +190,7 @@ class MODULES_EXPORT DeferredTaskHandler final ...@@ -190,7 +190,7 @@ class MODULES_EXPORT DeferredTaskHandler final
return &active_source_handlers_; return &active_source_handlers_;
} }
Vector<AudioHandler*>* GetFinishedSourceHandlers() { Vector<scoped_refptr<AudioHandler>>* GetFinishedSourceHandlers() {
return &finished_source_handlers_; return &finished_source_handlers_;
} }
...@@ -259,7 +259,7 @@ class MODULES_EXPORT DeferredTaskHandler final ...@@ -259,7 +259,7 @@ class MODULES_EXPORT DeferredTaskHandler final
// connection and elements here are removed from |active_source_handlers_|. // connection and elements here are removed from |active_source_handlers_|.
// //
// This must be accessed only from the audio thread. // This must be accessed only from the audio thread.
Vector<AudioHandler*> finished_source_handlers_; Vector<scoped_refptr<AudioHandler>> finished_source_handlers_;
scoped_refptr<base::SingleThreadTaskRunner> task_runner_; scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment