Fix race with Asset destruction and printing allocation stats (882f8168) · Commits · e / os / android_frameworks_base

include/androidfw/Asset.h

+14 −1

Original line number	Diff line number	Diff line
		@@ -44,7 +44,7 @@ namespace android {
		*/
		class Asset {
		public:
		virtual ~Asset(void);
		virtual ~Asset(void) = default;

		static int32_t getGlobalCount();
		static String8 getAssetAllocations();
		@@ -119,6 +119,19 @@ public:
		const char* getAssetSource(void) const { return mAssetSource.string(); }

		protected:
		/*
		* Adds this Asset to the global Asset list for debugging and
		* accounting.
		* Concrete subclasses must call this in their constructor.
		*/
		static void registerAsset(Asset* asset);

		/*
		* Removes this Asset from the global Asset list.
		* Concrete subclasses must call this in their destructor.
		*/
		static void unregisterAsset(Asset* asset);

		Asset(void); // constructor; only invoked indirectly

		/* handle common seek() housekeeping */

libs/androidfw/Asset.cpp

+56 −36

Original line number	Diff line number	Diff line
		@@ -52,6 +52,47 @@ static int32_t gCount = 0;
		static Asset* gHead = NULL;
		static Asset* gTail = NULL;

		void Asset::registerAsset(Asset* asset)
		{
		AutoMutex _l(gAssetLock);
		gCount++;
		asset->mNext = asset->mPrev = NULL;
		if (gTail == NULL) {
		gHead = gTail = asset;
		} else {
		asset->mPrev = gTail;
		gTail->mNext = asset;
		gTail = asset;
		}

		if (kIsDebug) {
		ALOGI("Creating Asset %p #%d\n", asset, gCount);
		}
		}

		void Asset::unregisterAsset(Asset* asset)
		{
		AutoMutex _l(gAssetLock);
		gCount--;
		if (gHead == asset) {
		gHead = asset->mNext;
		}
		if (gTail == asset) {
		gTail = asset->mPrev;
		}
		if (asset->mNext != NULL) {
		asset->mNext->mPrev = asset->mPrev;
		}
		if (asset->mPrev != NULL) {
		asset->mPrev->mNext = asset->mNext;
		}
		asset->mNext = asset->mPrev = NULL;

		if (kIsDebug) {
		ALOGI("Destroying Asset in %p #%d\n", asset, gCount);
		}
		}

		int32_t Asset::getGlobalCount()
		{
		AutoMutex _l(gAssetLock);
		@@ -79,43 +120,8 @@ String8 Asset::getAssetAllocations()
		}

		Asset::Asset(void)
		: mAccessMode(ACCESS_UNKNOWN)
		: mAccessMode(ACCESS_UNKNOWN), mNext(NULL), mPrev(NULL)
		{
		AutoMutex _l(gAssetLock);
		gCount++;
		mNext = mPrev = NULL;
		if (gTail == NULL) {
		gHead = gTail = this;
		} else {
		mPrev = gTail;
		gTail->mNext = this;
		gTail = this;
		}
		if (kIsDebug) {
		ALOGI("Creating Asset %p #%d\n", this, gCount);
		}
		}

		Asset::~Asset(void)
		{
		AutoMutex _l(gAssetLock);
		gCount--;
		if (gHead == this) {
		gHead = mNext;
		}
		if (gTail == this) {
		gTail = mPrev;
		}
		if (mNext != NULL) {
		mNext->mPrev = mPrev;
		}
		if (mPrev != NULL) {
		mPrev->mNext = mNext;
		}
		mNext = mPrev = NULL;
		if (kIsDebug) {
		ALOGI("Destroying Asset in %p #%d\n", this, gCount);
		}
		}

		/*
		@@ -361,6 +367,9 @@ off64_t Asset::handleSeek(off64_t offset, int whence, off64_t curPosn, off64_t m
		_FileAsset::_FileAsset(void)
		: mStart(0), mLength(0), mOffset(0), mFp(NULL), mFileName(NULL), mMap(NULL), mBuf(NULL)
		{
		// Register the Asset with the global list here after it is fully constructed and its
		// vtable pointer points to this concrete type. b/31113965
		registerAsset(this);
		}

		/*
		@@ -369,6 +378,10 @@ _FileAsset::_FileAsset(void)
		_FileAsset::~_FileAsset(void)
		{
		close();

		// Unregister the Asset from the global list here before it is destructed and while its vtable
		// pointer still points to this concrete type. b/31113965
		unregisterAsset(this);
		}

		/*
		@@ -685,6 +698,9 @@ _CompressedAsset::_CompressedAsset(void)
		: mStart(0), mCompressedLen(0), mUncompressedLen(0), mOffset(0),
		mMap(NULL), mFd(-1), mZipInflater(NULL), mBuf(NULL)
		{
		// Register the Asset with the global list here after it is fully constructed and its
		// vtable pointer points to this concrete type. b/31113965
		registerAsset(this);
		}

		/*
		@@ -693,6 +709,10 @@ _CompressedAsset::_CompressedAsset(void)
		_CompressedAsset::~_CompressedAsset(void)
		{
		close();

		// Unregister the Asset from the global list here before it is destructed and while its vtable
		// pointer still points to this concrete type. b/31113965
		unregisterAsset(this);
		}

		/*

libs/androidfw/tests/Android.mk

+1 −0

Original line number	Diff line number	Diff line
		@@ -22,6 +22,7 @@ LOCAL_PATH:= $(call my-dir)

		testFiles := \
		AppAsLib_test.cpp \
		Asset_test.cpp \
		AttributeFinder_test.cpp \
		ByteBucketArray_test.cpp \
		Config_test.cpp \

libs/androidfw/tests/Asset_test.cpp

0 → 100644

+37 −0

Original line number	Diff line number	Diff line
		/*
		* Copyright (C) 2016 The Android Open Source Project
		*
		* Licensed under the Apache License, Version 2.0 (the "License");
		* you may not use this file except in compliance with the License.
		* You may obtain a copy of the License at
		*
		* http://www.apache.org/licenses/LICENSE-2.0
		*
		* Unless required by applicable law or agreed to in writing, software
		* distributed under the License is distributed on an "AS IS" BASIS,
		* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
		* See the License for the specific language governing permissions and
		* limitations under the License.
		*/

		#include <androidfw/Asset.h>

		#include <gtest/gtest.h>

		using namespace android;

		TEST(AssetTest, FileAssetRegistersItself) {
		const int32_t count = Asset::getGlobalCount();
		Asset* asset = new _FileAsset();
		EXPECT_EQ(count + 1, Asset::getGlobalCount());
		delete asset;
		EXPECT_EQ(count, Asset::getGlobalCount());
		}

		TEST(AssetTest, CompressedAssetRegistersItself) {
		const int32_t count = Asset::getGlobalCount();
		Asset* asset = new _CompressedAsset();
		EXPECT_EQ(count + 1, Asset::getGlobalCount());
		delete asset;
		EXPECT_EQ(count, Asset::getGlobalCount());
		}