Skip to content

Commit

Permalink
Fix leaks in WebAudio layout tests
Browse files Browse the repository at this point in the history
When the context is closing, the nodes that were processing their
tails need to be stopped.  This disables the output of the node, which
might cause other nodes to disable their outputs, and these nodes are
placed on |finished_tail_processing_handlers_|.  These also need to be
handled when finishing tail processing.  So loop around processing
|finished_tail_processing_handlers_| and |tail_processing_handlers_|
until disabling of outputs no longer adds anything to either of these.


Bug: 832043, 828826
Test: run-webkit-tests --enable-leak-detection no longer detects leaks
Change-Id: I6ca195352b240d343095c208c9b70b0c5553c925
Reviewed-on: https://chromium-review.googlesource.com/1010833
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550468}
  • Loading branch information
Raymond Toy authored and Commit Bot committed Apr 13, 2018
1 parent 758bf1b commit cecfc61
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -328,14 +328,7 @@ void DeferredTaskHandler::DeleteHandlersOnMainThread() {
DCHECK(IsMainThread());
GraphAutoLocker locker(*this);
deletable_orphan_handlers_.clear();
// Tail processing nodes have finished processing their tails so we need to
// disable their outputs to indicate to downstream nodes that they're done.
// This has to be done in the main thread because DisableOutputs() can causing
// summing juctions to go away, which must be done on the main thread.
for (auto& handler : finished_tail_processing_handlers_) {
handler->DisableOutputs();
}
finished_tail_processing_handlers_.clear();
DisableOutputsForTailProcessing();
}

void DeferredTaskHandler::ClearHandlersToBeDeleted() {
Expand All @@ -352,21 +345,44 @@ void DeferredTaskHandler::SetAudioThreadToCurrentThread() {
ReleaseStore(&audio_thread_, thread);
}

void DeferredTaskHandler::DisableOutputsForTailProcessing() {
DCHECK(IsMainThread());
// Tail processing nodes have finished processing their tails so we need to
// disable their outputs to indicate to downstream nodes that they're done.
// This has to be done in the main thread because DisableOutputs() can cause
// summing juctions to go away, which must be done on the main thread.
for (auto& handler : finished_tail_processing_handlers_) {
handler->DisableOutputs();
}
finished_tail_processing_handlers_.clear();
}

void DeferredTaskHandler::FinishTailProcessing() {
DCHECK(IsMainThread());
// DisableOutputs must run with the graph lock.
GraphAutoLocker locker(*this);

while (tail_processing_handlers_.size() > 0) {
// |DisableOutputs()| can modify |tail_processing_handlers_|, so
// swap it out before processing it. And keep running this until
// nothing gets added to |tail_processing_handlers_|.
Vector<scoped_refptr<AudioHandler>> handlers;

handlers.swap(tail_processing_handlers_);
for (auto& handler : handlers)
handler->DisableOutputs();
}
// TODO(crbug.com/832200): Simplify this!

// |DisableOutputs()| can cause new handlers to start tail processing, which
// in turn can cause hte handler to want to disable outputs. For the former
// case, the handler is added to |tail_processing_handlers_|. In the latter
// case, the handler is added to |finished_tail_processing_handlers_|. So, we
// need to loop around until these vectors are completely empty.
do {
while (tail_processing_handlers_.size() > 0) {
// |DisableOutputs()| can modify |tail_processing_handlers_|, so
// swap it out before processing it. And keep running this until
// nothing gets added to |tail_processing_handlers_|.
Vector<scoped_refptr<AudioHandler>> handlers_to_be_disabled;

handlers_to_be_disabled.swap(tail_processing_handlers_);
for (auto& handler : handlers_to_be_disabled)
handler->DisableOutputs();
}
DisableOutputsForTailProcessing();
} while (tail_processing_handlers_.size() > 0 ||
finished_tail_processing_handlers_.size() > 0);
}

} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ class MODULES_EXPORT DeferredTaskHandler final
// context is done.
void FinishTailProcessing();

// For handlers that have finished processing their tail and require disabling
// the ouputs of the handler, we do that here.
void DisableOutputsForTailProcessing();

//
// Thread Safety and Graph Locking:
//
Expand Down

0 comments on commit cecfc61

Please sign in to comment.