Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 6209d30

Browse files
committed
Do not attempt to drain the SkiaUnrefQueue in the destructor
SkiaUnrefQueue should be empty at destruction time. If the queue is nonempty, then there will be a pending drain task that will hold a reference to the queue. The queue can only be destructed after the drain completes and the reference is dropped. Drains must only be done on the queue's task runner thread, which may not be the thread where the queue is destructed.
1 parent 8882bf3 commit 6209d30

File tree

4 files changed

+56
-3
lines changed

4 files changed

+56
-3
lines changed

flow/BUILD.gn

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,13 +114,15 @@ executable("flow_unittests") {
114114
"matrix_decomposition_unittests.cc",
115115
"mutators_stack_unittests.cc",
116116
"raster_cache_unittests.cc",
117+
"skia_gpu_object_unittests.cc",
117118
"texture_unittests.cc",
118119
]
119120

120121
deps = [
121122
":flow",
122123
":flow_fixtures",
123124
"$flutter_root/fml",
125+
"$flutter_root/testing:testing_lib",
124126
"//third_party/dart/runtime:libdart_jit", # for tracing
125127
"//third_party/googletest:gtest",
126128
"//third_party/skia",

flow/skia_gpu_object.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ SkiaUnrefQueue::SkiaUnrefQueue(fml::RefPtr<fml::TaskRunner> task_runner,
1515
drain_pending_(false) {}
1616

1717
SkiaUnrefQueue::~SkiaUnrefQueue() {
18-
Drain();
18+
FML_DCHECK(objects_.empty());
1919
}
2020

2121
void SkiaUnrefQueue::Unref(SkRefCnt* object) {

flow/skia_gpu_object_unittests.cc

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "flutter/flow/skia_gpu_object.h"
6+
#include "flutter/fml/message_loop.h"
7+
#include "flutter/fml/synchronization/waitable_event.h"
8+
#include "flutter/fml/task_runner.h"
9+
#include "flutter/testing/thread_test.h"
10+
#include "gtest/gtest.h"
11+
#include "third_party/skia/include/core/SkRefCnt.h"
12+
13+
namespace flutter {
14+
namespace testing {
15+
16+
using SkiaGpuObjectTest = flutter::testing::ThreadTest;
17+
18+
class TestSkObject : public SkRefCnt {
19+
public:
20+
TestSkObject(std::shared_ptr<fml::AutoResetWaitableEvent> latch,
21+
fml::TaskQueueId* dtor_task_queue_id)
22+
: latch_(latch), dtor_task_queue_id_(dtor_task_queue_id) {}
23+
24+
~TestSkObject() {
25+
FML_LOG(ERROR) << "***** " << __PRETTY_FUNCTION__;
26+
*dtor_task_queue_id_ = fml::MessageLoop::GetCurrentTaskQueueId();
27+
latch_->Signal();
28+
}
29+
30+
private:
31+
std::shared_ptr<fml::AutoResetWaitableEvent> latch_;
32+
fml::TaskQueueId* dtor_task_queue_id_;
33+
};
34+
35+
TEST_F(SkiaGpuObjectTest, UnrefQueue) {
36+
fml::RefPtr<fml::TaskRunner> task_runner = CreateNewThread();
37+
fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>(
38+
task_runner, fml::TimeDelta::FromSeconds(0));
39+
40+
std::shared_ptr<fml::AutoResetWaitableEvent> latch =
41+
std::make_shared<fml::AutoResetWaitableEvent>();
42+
fml::TaskQueueId dtor_task_queue_id(0);
43+
SkRefCnt* ref_object = new TestSkObject(latch, &dtor_task_queue_id);
44+
45+
queue->Unref(ref_object);
46+
latch->Wait();
47+
ASSERT_EQ(dtor_task_queue_id, task_runner->GetTaskQueueId());
48+
}
49+
50+
} // namespace testing
51+
} // namespace flutter

testing/BUILD.gn

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ source_set("testing_lib") {
99
"$flutter_root/testing/assertions.h",
1010
"$flutter_root/testing/testing.cc",
1111
"$flutter_root/testing/testing.h",
12+
"$flutter_root/testing/thread_test.cc",
13+
"$flutter_root/testing/thread_test.h",
1214
]
1315

1416
public_deps = [
@@ -23,8 +25,6 @@ source_set("testing") {
2325

2426
sources = [
2527
"$flutter_root/testing/run_all_unittests.cc",
26-
"$flutter_root/testing/thread_test.cc",
27-
"$flutter_root/testing/thread_test.h",
2828
]
2929

3030
public_deps = [

0 commit comments

Comments
 (0)