[mediacodec] Fix CodecAdapterSW shutdown order
Sometimes a member held references to a member that destructed earlier.
TEST: fx run-test codec_adapter_sw_test
Change-Id: I4ad8dc14d562aac45aa7e53963b3a7d4f6c33a43
diff --git a/garnet/bin/media/codecs/sw/codec_adapter_sw.h b/garnet/bin/media/codecs/sw/codec_adapter_sw.h
index 263642e..4c21cb2 100644
--- a/garnet/bin/media/codecs/sw/codec_adapter_sw.h
+++ b/garnet/bin/media/codecs/sw/codec_adapter_sw.h
@@ -336,8 +336,8 @@
BlockingMpscQueue<CodecInputItem> input_queue_;
BlockingMpscQueue<CodecPacket*> free_output_packets_;
- std::map<CodecPacket*, LocalOutput> in_use_by_client_ FXL_GUARDED_BY(lock_);
BufferPool output_buffer_pool_;
+ std::map<CodecPacket*, LocalOutput> in_use_by_client_ FXL_GUARDED_BY(lock_);
// Buffers the client has added but that we cannot use until configuration is
// complete.
diff --git a/garnet/bin/media/codecs/test/BUILD.gn b/garnet/bin/media/codecs/test/BUILD.gn
index 1998b64..2b9ddc4 100644
--- a/garnet/bin/media/codecs/test/BUILD.gn
+++ b/garnet/bin/media/codecs/test/BUILD.gn
@@ -10,6 +10,7 @@
testonly = true
deps = [
":chunk_input_stream_tests",
+ ":codec_adapter_sw_test",
":mpsc_queue_tests",
":output_sink_tests",
":raw_frames",
@@ -20,6 +21,33 @@
]
}
+executable("codec_adapter_sw_test_bin") {
+ output_name = "codec_adapter_sw_test"
+ testonly = true
+
+ sources = [
+ "codec_adapter_sw_test.cc",
+ ]
+
+ deps = [
+ "//garnet/bin/media/codecs/sw:codec_adapter_sw",
+ "//src/lib/fxl/test:gtest_main",
+ ]
+}
+
+test_package("codec_adapter_sw_test") {
+ tests = [
+ {
+ name = "codec_adapter_sw_test"
+ environments = basic_envs
+ },
+ ]
+
+ deps = [
+ ":codec_adapter_sw_test_bin",
+ ]
+}
+
source_set("raw_frames") {
testonly = true
diff --git a/garnet/bin/media/codecs/test/codec_adapter_sw_test.cc b/garnet/bin/media/codecs/test/codec_adapter_sw_test.cc
new file mode 100644
index 0000000..127a73ea
--- /dev/null
+++ b/garnet/bin/media/codecs/test/codec_adapter_sw_test.cc
@@ -0,0 +1,63 @@
+// Copyright 2019 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "../sw/codec_adapter_sw.h"
+
+#include <lib/fit/defer.h>
+#include <lib/fit/function.h>
+
+#include "gtest/gtest.h"
+
+class CodecAdapterSWDummy
+ : public CodecAdapterSW<fit::deferred_action<fit::closure>> {
+ public:
+ CodecAdapterSWDummy(std::mutex& lock)
+ : CodecAdapterSW(lock,
+ /* bad ptr to pass non-null assert */ reinterpret_cast<
+ CodecAdapterEvents*>(0xaa)) {}
+
+ // Much like the real fit::defer(s) in in_use_by_client_, the fit::defer we're
+ // putting in in_use_by_client_ touches output_buffer_pool_ in a way that'll
+ // crash if output_buffer_pool_ is already destructed.
+ void EntangleClientMapAndBufferPoolDestructors() {
+ std::lock_guard<std::mutex> lock(lock_);
+ fit::closure deferred = [this]() {
+ // This call will crash if output_buffer_pool_ has already been
+ // destructed, much like the real-world fit::defer(s) that would be in
+ // in_use_by_client_ if we delete CodecAdapterSW with stuff in flight.
+ output_buffer_pool_.has_buffers_in_use();
+ };
+ in_use_by_client_[nullptr] = fit::defer(std::move(deferred));
+ }
+
+ fuchsia::sysmem::BufferCollectionConstraints
+ CoreCodecGetBufferCollectionConstraints(
+ CodecPort port,
+ const fuchsia::media::StreamBufferConstraints& stream_buffer_constraints,
+ const fuchsia::media::StreamBufferPartialSettings& partial_settings)
+ override {
+ return fuchsia::sysmem::BufferCollectionConstraints();
+ }
+
+ void CoreCodecSetBufferCollectionInfo(
+ CodecPort port,
+ const fuchsia::sysmem::BufferCollectionInfo_2& buffer_collection_info)
+ override {}
+
+ protected:
+ virtual void ProcessInputLoop() override {}
+
+ virtual void CleanUpAfterStream() override {}
+
+ virtual std::pair<fuchsia::media::FormatDetails, size_t> OutputFormatDetails()
+ override {
+ return {fuchsia::media::FormatDetails(), 0};
+ }
+};
+
+TEST(CodecAdapterSW, DoesNotCrashOnDestruction) {
+ // To pass, this test must not crash.
+ std::mutex lock;
+ auto under_test = CodecAdapterSWDummy(lock);
+}
diff --git a/garnet/bin/media/codecs/test/meta/codec_adapter_sw_test.cmx b/garnet/bin/media/codecs/test/meta/codec_adapter_sw_test.cmx
new file mode 100644
index 0000000..8bcb07f
--- /dev/null
+++ b/garnet/bin/media/codecs/test/meta/codec_adapter_sw_test.cmx
@@ -0,0 +1,9 @@
+{
+ "program": {
+ "binary": "test/codec_adapter_sw_test"
+ },
+ "sandbox": {
+ "features": [],
+ "services": []
+ }
+}
