diff options
author | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2023-08-11 21:06:24 +0000 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2023-08-11 21:06:24 +0000 |
commit | 9d7c2e683f6116c6e9577d5116e4e4563df19ff3 (patch) | |
tree | 4075936ce82123a2572f0551ae1ee4bca8e271ba | |
parent | a4a81a34c7ec96e5eb23b81e358e5d7264ddb9be (diff) | |
parent | 52d9d50f81e9803fe665662db18e33989a73bc55 (diff) | |
download | native-android-platform-12.1.0_r21.tar.gz |
Merge cherrypicks of ['googleplex-android-review.googlesource.com/23905808'] into sc-v2-platform-release.android-platform-12.1.0_r21android-platform-12.1.0_r20
Change-Id: I7bdc15365fa8f285cdf96283a59188ee0015d753
-rw-r--r-- | services/gpuservice/Android.bp | 1 | ||||
-rw-r--r-- | services/gpuservice/GpuService.cpp | 10 | ||||
-rw-r--r-- | services/gpuservice/include/gpuservice/GpuService.h (renamed from services/gpuservice/GpuService.h) | 3 | ||||
-rw-r--r-- | services/gpuservice/main_gpuservice.cpp | 2 | ||||
-rw-r--r-- | services/gpuservice/tests/unittests/Android.bp | 2 | ||||
-rw-r--r-- | services/gpuservice/tests/unittests/GpuServiceTest.cpp | 52 |
6 files changed, 66 insertions, 4 deletions
diff --git a/services/gpuservice/Android.bp b/services/gpuservice/Android.bp index b9b6a19606..8cbfa6fabe 100644 --- a/services/gpuservice/Android.bp +++ b/services/gpuservice/Android.bp @@ -70,6 +70,7 @@ filegroup { cc_library_shared { name: "libgpuservice", defaults: ["libgpuservice_production_defaults"], + export_include_dirs: ["include"], srcs: [ ":libgpuservice_sources", ], diff --git a/services/gpuservice/GpuService.cpp b/services/gpuservice/GpuService.cpp index 52d5d4fc46..44af4c4877 100644 --- a/services/gpuservice/GpuService.cpp +++ b/services/gpuservice/GpuService.cpp @@ -16,7 +16,7 @@ #define ATRACE_TAG ATRACE_TAG_GRAPHICS -#include "GpuService.h" +#include "gpuservice/GpuService.h" #include <android-base/stringprintf.h> #include <binder/IPCThreadState.h> @@ -33,6 +33,7 @@ #include <vkjson.h> #include <thread> +#include <memory> namespace android { @@ -52,13 +53,16 @@ GpuService::GpuService() : mGpuMem(std::make_shared<GpuMem>()), mGpuStats(std::make_unique<GpuStats>()), mGpuMemTracer(std::make_unique<GpuMemTracer>()) { - std::thread asyncInitThread([this]() { + mGpuMemAsyncInitThread = std::make_unique<std::thread>([this] (){ mGpuMem->initialize(); mGpuMemTracer->initialize(mGpuMem); }); - asyncInitThread.detach(); }; +GpuService::~GpuService() { + mGpuMemAsyncInitThread->join(); +} + void GpuService::setGpuStats(const std::string& driverPackageName, const std::string& driverVersionName, uint64_t driverVersionCode, int64_t driverBuildTime, const std::string& appPackageName, diff --git a/services/gpuservice/GpuService.h b/services/gpuservice/include/gpuservice/GpuService.h index 409084b656..7d56c83890 100644 --- a/services/gpuservice/GpuService.h +++ b/services/gpuservice/include/gpuservice/GpuService.h @@ -24,6 +24,7 @@ #include <serviceutils/PriorityDumper.h> #include <mutex> +#include <thread> #include <vector> namespace android { @@ -37,6 +38,7 @@ public: static const char* const SERVICE_NAME ANDROID_API; GpuService() ANDROID_API; + ~GpuService(); protected: status_t shellCommand(int in, int out, int err, std::vector<String16>& args) override; @@ -81,6 +83,7 @@ private: std::unique_ptr<GpuMemTracer> mGpuMemTracer; std::mutex mLock; std::string mDeveloperDriverPath; + std::unique_ptr<std::thread> mGpuMemAsyncInitThread; }; } // namespace android diff --git a/services/gpuservice/main_gpuservice.cpp b/services/gpuservice/main_gpuservice.cpp index 64aafcab6a..200237219e 100644 --- a/services/gpuservice/main_gpuservice.cpp +++ b/services/gpuservice/main_gpuservice.cpp @@ -18,7 +18,7 @@ #include <binder/IServiceManager.h> #include <binder/ProcessState.h> #include <sys/resource.h> -#include "GpuService.h" +#include "gpuservice/GpuService.h" using namespace android; diff --git a/services/gpuservice/tests/unittests/Android.bp b/services/gpuservice/tests/unittests/Android.bp index 6d87c45921..1455d2eddb 100644 --- a/services/gpuservice/tests/unittests/Android.bp +++ b/services/gpuservice/tests/unittests/Android.bp @@ -31,6 +31,7 @@ cc_test { "GpuMemTest.cpp", "GpuMemTracerTest.cpp", "GpuStatsTest.cpp", + "GpuServiceTest.cpp", ], shared_libs: [ "libbase", @@ -47,6 +48,7 @@ cc_test { "libstatslog", "libstatspull", "libutils", + "libgpuservice", ], static_libs: [ "libgmock", diff --git a/services/gpuservice/tests/unittests/GpuServiceTest.cpp b/services/gpuservice/tests/unittests/GpuServiceTest.cpp new file mode 100644 index 0000000000..62b3e53f53 --- /dev/null +++ b/services/gpuservice/tests/unittests/GpuServiceTest.cpp @@ -0,0 +1,52 @@ +#undef LOG_TAG +#define LOG_TAG "gpuservice_unittest" + +#include "gpuservice/GpuService.h" + +#include <gtest/gtest.h> +#include <log/log_main.h> + +#include <chrono> +#include <thread> + +namespace android { +namespace { + +class GpuServiceTest : public testing::Test { +public: + GpuServiceTest() { + const ::testing::TestInfo* const test_info = + ::testing::UnitTest::GetInstance()->current_test_info(); + ALOGD("**** Setting up for %s.%s\n", test_info->test_case_name(), test_info->name()); + } + + ~GpuServiceTest() { + const ::testing::TestInfo* const test_info = + ::testing::UnitTest::GetInstance()->current_test_info(); + ALOGD("**** Tearing down after %s.%s\n", test_info->test_case_name(), test_info->name()); + } + +}; + + +/* +* The behaviour before this test + fixes was UB caused by threads accessing deallocated memory. +* +* This test creates the service (which initializes the culprit threads), +* deallocates it immediately and sleeps. +* +* GpuService's destructor gets called and joins the threads. +* If we haven't crashed by the time the sleep time has elapsed, we're good +* Let the test pass. +*/ +TEST_F(GpuServiceTest, onInitializeShouldNotCauseUseAfterFree) { + sp<GpuService> service = new GpuService(); + service.clear(); + std::this_thread::sleep_for(std::chrono::seconds(3)); + + // If we haven't crashed yet due to threads accessing freed up memory, let the test pass + EXPECT_TRUE(true); +} + +} // namespace +} // namespace android |