RESTRICT AUTOMERGE Validate TrackedBuffer in onBufferDestroyed am: c155398cd7 (892a451f) · Commits · e / os / android_frameworks_av

media/codec2/hidl/1.0/utils/InputBufferManager.cpp

+64 −49

Original line number	Original line	Diff line number	Diff line
	@@ -70,7 +70,7 @@ void InputBufferManager::_registerFrameData(
	<< ".";		<< ".";
	std::lock_guard<std::mutex> lock(mMutex);		std::lock_guard<std::mutex> lock(mMutex);

	std::set<TrackedBuffer> &bufferIds =		std::set<TrackedBuffer*> &bufferIds =
	mTrackedBuffersMap[listener][frameIndex];		mTrackedBuffersMap[listener][frameIndex];

	for (size_t i = 0; i < input.buffers.size(); ++i) {		for (size_t i = 0; i < input.buffers.size(); ++i) {
	@@ -79,13 +79,14 @@ void InputBufferManager::_registerFrameData(
	<< "Input buffer at index " << i << " is null.";		<< "Input buffer at index " << i << " is null.";
	continue;		continue;
	}		}
	const TrackedBuffer &bufferId =		TrackedBuffer *bufferId =
	*bufferIds.emplace(listener, frameIndex, i, input.buffers[i]).		new TrackedBuffer(listener, frameIndex, i, input.buffers[i]);
	first;		mTrackedBufferCache.emplace(bufferId);
			bufferIds.emplace(bufferId);

	c2_status_t status = input.buffers[i]->registerOnDestroyNotify(		c2_status_t status = input.buffers[i]->registerOnDestroyNotify(
	onBufferDestroyed,		onBufferDestroyed,
	const_cast<void>(reinterpret_cast<const void>(&bufferId)));		reinterpret_cast<void*>(bufferId));
	if (status != C2_OK) {		if (status != C2_OK) {
	LOG(DEBUG) << "InputBufferManager::_registerFrameData -- "		LOG(DEBUG) << "InputBufferManager::_registerFrameData -- "
	<< "registerOnDestroyNotify() failed "		<< "registerOnDestroyNotify() failed "
	@@ -119,31 +120,32 @@ void InputBufferManager::_unregisterFrameData(

	auto findListener = mTrackedBuffersMap.find(listener);		auto findListener = mTrackedBuffersMap.find(listener);
	if (findListener != mTrackedBuffersMap.end()) {		if (findListener != mTrackedBuffersMap.end()) {
	std::map<uint64_t, std::set<TrackedBuffer>> &frameIndex2BufferIds		std::map<uint64_t, std::set<TrackedBuffer*>> &frameIndex2BufferIds
	= findListener->second;		= findListener->second;
	auto findFrameIndex = frameIndex2BufferIds.find(frameIndex);		auto findFrameIndex = frameIndex2BufferIds.find(frameIndex);
	if (findFrameIndex != frameIndex2BufferIds.end()) {		if (findFrameIndex != frameIndex2BufferIds.end()) {
	std::set<TrackedBuffer> &bufferIds = findFrameIndex->second;		std::set<TrackedBuffer*> &bufferIds = findFrameIndex->second;
	for (const TrackedBuffer& bufferId : bufferIds) {		for (TrackedBuffer* bufferId : bufferIds) {
	std::shared_ptr<C2Buffer> buffer = bufferId.buffer.lock();		std::shared_ptr<C2Buffer> buffer = bufferId->buffer.lock();
	if (buffer) {		if (buffer) {
	c2_status_t status = buffer->unregisterOnDestroyNotify(		c2_status_t status = buffer->unregisterOnDestroyNotify(
	onBufferDestroyed,		onBufferDestroyed,
	const_cast<void*>(		reinterpret_cast<void*>(bufferId));
	reinterpret_cast<const void*>(&bufferId)));
	if (status != C2_OK) {		if (status != C2_OK) {
	LOG(DEBUG) << "InputBufferManager::_unregisterFrameData "		LOG(DEBUG) << "InputBufferManager::_unregisterFrameData "
	<< "-- unregisterOnDestroyNotify() failed "		<< "-- unregisterOnDestroyNotify() failed "
	<< "(listener @ 0x"		<< "(listener @ 0x"
	<< std::hex		<< std::hex
	<< bufferId.listener.unsafe_get()		<< bufferId->listener.unsafe_get()
	<< ", frameIndex = "		<< ", frameIndex = "
	<< std::dec << bufferId.frameIndex		<< std::dec << bufferId->frameIndex
	<< ", bufferIndex = " << bufferId.bufferIndex		<< ", bufferIndex = " << bufferId->bufferIndex
	<< ") => status = " << status		<< ") => status = " << status
	<< ".";		<< ".";
	}		}
	}		}
			mTrackedBufferCache.erase(bufferId);
			delete bufferId;
	}		}

	frameIndex2BufferIds.erase(findFrameIndex);		frameIndex2BufferIds.erase(findFrameIndex);
	@@ -179,31 +181,32 @@ void InputBufferManager::_unregisterFrameData(

	auto findListener = mTrackedBuffersMap.find(listener);		auto findListener = mTrackedBuffersMap.find(listener);
	if (findListener != mTrackedBuffersMap.end()) {		if (findListener != mTrackedBuffersMap.end()) {
	std::map<uint64_t, std::set<TrackedBuffer>> &frameIndex2BufferIds =		std::map<uint64_t, std::set<TrackedBuffer*>> &frameIndex2BufferIds =
	findListener->second;		findListener->second;
	for (auto findFrameIndex = frameIndex2BufferIds.begin();		for (auto findFrameIndex = frameIndex2BufferIds.begin();
	findFrameIndex != frameIndex2BufferIds.end();		findFrameIndex != frameIndex2BufferIds.end();
	++findFrameIndex) {		++findFrameIndex) {
	std::set<TrackedBuffer> &bufferIds = findFrameIndex->second;		std::set<TrackedBuffer*> &bufferIds = findFrameIndex->second;
	for (const TrackedBuffer& bufferId : bufferIds) {		for (TrackedBuffer* bufferId : bufferIds) {
	std::shared_ptr<C2Buffer> buffer = bufferId.buffer.lock();		std::shared_ptr<C2Buffer> buffer = bufferId->buffer.lock();
	if (buffer) {		if (buffer) {
	c2_status_t status = buffer->unregisterOnDestroyNotify(		c2_status_t status = buffer->unregisterOnDestroyNotify(
	onBufferDestroyed,		onBufferDestroyed,
	const_cast<void*>(		reinterpret_cast<void*>(bufferId));
	reinterpret_cast<const void*>(&bufferId)));
	if (status != C2_OK) {		if (status != C2_OK) {
	LOG(DEBUG) << "InputBufferManager::_unregisterFrameData "		LOG(DEBUG) << "InputBufferManager::_unregisterFrameData "
	<< "-- unregisterOnDestroyNotify() failed "		<< "-- unregisterOnDestroyNotify() failed "
	<< "(listener @ 0x"		<< "(listener @ 0x"
	<< std::hex		<< std::hex
	<< bufferId.listener.unsafe_get()		<< bufferId->listener.unsafe_get()
	<< ", frameIndex = "		<< ", frameIndex = "
	<< std::dec << bufferId.frameIndex		<< std::dec << bufferId->frameIndex
	<< ", bufferIndex = " << bufferId.bufferIndex		<< ", bufferIndex = " << bufferId->bufferIndex
	<< ") => status = " << status		<< ") => status = " << status
	<< ".";		<< ".";
	}		}
			mTrackedBufferCache.erase(bufferId);
			delete bufferId;
	}		}
	}		}
	}		}
	@@ -236,50 +239,59 @@ void InputBufferManager::_onBufferDestroyed(const C2Buffer* buf, void* arg) {
	<< std::dec << ".";		<< std::dec << ".";
	return;		return;
	}		}
	TrackedBuffer id(reinterpret_cast<TrackedBuffer>(arg));
			std::lock_guard<std::mutex> lock(mMutex);
			TrackedBuffer bufferId = reinterpret_cast<TrackedBuffer>(arg);

			if (mTrackedBufferCache.find(bufferId) == mTrackedBufferCache.end()) {
			LOG(VERBOSE) << "InputBufferManager::_onBufferDestroyed -- called with "
			<< "unregistered buffer: "
			<< "buf @ 0x" << std::hex << buf
			<< ", arg @ 0x" << std::hex << arg
			<< std::dec << ".";
			return;
			}

	LOG(VERBOSE) << "InputBufferManager::_onBufferDestroyed -- called with "		LOG(VERBOSE) << "InputBufferManager::_onBufferDestroyed -- called with "
	<< "buf @ 0x" << std::hex << buf		<< "buf @ 0x" << std::hex << buf
	<< ", arg @ 0x" << std::hex << arg		<< ", arg @ 0x" << std::hex << arg
	<< std::dec << " -- "		<< std::dec << " -- "
	<< "listener @ 0x" << std::hex << id.listener.unsafe_get()		<< "listener @ 0x" << std::hex << bufferId->listener.unsafe_get()
	<< ", frameIndex = " << std::dec << id.frameIndex		<< ", frameIndex = " << std::dec << bufferId->frameIndex
	<< ", bufferIndex = " << id.bufferIndex		<< ", bufferIndex = " << bufferId->bufferIndex
	<< ".";		<< ".";
			auto findListener = mTrackedBuffersMap.find(bufferId->listener);
	std::lock_guard<std::mutex> lock(mMutex);

	auto findListener = mTrackedBuffersMap.find(id.listener);
	if (findListener == mTrackedBuffersMap.end()) {		if (findListener == mTrackedBuffersMap.end()) {
	LOG(DEBUG) << "InputBufferManager::_onBufferDestroyed -- "		LOG(VERBOSE) << "InputBufferManager::_onBufferDestroyed -- "
	<< "received invalid listener: "		<< "received invalid listener: "
	<< "listener @ 0x" << std::hex << id.listener.unsafe_get()		<< "listener @ 0x" << std::hex << bufferId->listener.unsafe_get()
	<< " (frameIndex = " << std::dec << id.frameIndex		<< " (frameIndex = " << std::dec << bufferId->frameIndex
	<< ", bufferIndex = " << id.bufferIndex		<< ", bufferIndex = " << bufferId->bufferIndex
	<< ").";		<< ").";
	return;		return;
	}		}

	std::map<uint64_t, std::set<TrackedBuffer>> &frameIndex2BufferIds		std::map<uint64_t, std::set<TrackedBuffer*>> &frameIndex2BufferIds
	= findListener->second;		= findListener->second;
	auto findFrameIndex = frameIndex2BufferIds.find(id.frameIndex);		auto findFrameIndex = frameIndex2BufferIds.find(bufferId->frameIndex);
	if (findFrameIndex == frameIndex2BufferIds.end()) {		if (findFrameIndex == frameIndex2BufferIds.end()) {
	LOG(DEBUG) << "InputBufferManager::_onBufferDestroyed -- "		LOG(DEBUG) << "InputBufferManager::_onBufferDestroyed -- "
	<< "received invalid frame index: "		<< "received invalid frame index: "
	<< "frameIndex = " << id.frameIndex		<< "frameIndex = " << bufferId->frameIndex
	<< " (listener @ 0x" << std::hex << id.listener.unsafe_get()		<< " (listener @ 0x" << std::hex << bufferId->listener.unsafe_get()
	<< ", bufferIndex = " << std::dec << id.bufferIndex		<< ", bufferIndex = " << std::dec << bufferId->bufferIndex
	<< ").";		<< ").";
	return;		return;
	}		}

	std::set<TrackedBuffer> &bufferIds = findFrameIndex->second;		std::set<TrackedBuffer*> &bufferIds = findFrameIndex->second;
	auto findBufferId = bufferIds.find(id);		auto findBufferId = bufferIds.find(bufferId);
	if (findBufferId == bufferIds.end()) {		if (findBufferId == bufferIds.end()) {
	LOG(DEBUG) << "InputBufferManager::_onBufferDestroyed -- "		LOG(DEBUG) << "InputBufferManager::_onBufferDestroyed -- "
	<< "received invalid buffer index: "		<< "received invalid buffer index: "
	<< "bufferIndex = " << id.bufferIndex		<< "bufferIndex = " << bufferId->bufferIndex
	<< " (frameIndex = " << id.frameIndex		<< " (frameIndex = " << bufferId->frameIndex
	<< ", listener @ 0x" << std::hex << id.listener.unsafe_get()		<< ", listener @ 0x" << std::hex << bufferId->listener.unsafe_get()
	<< std::dec << ").";		<< std::dec << ").";
	return;		return;
	}		}
	@@ -292,10 +304,13 @@ void InputBufferManager::_onBufferDestroyed(const C2Buffer* buf, void* arg) {
	}		}
	}		}

	DeathNotifications &deathNotifications = mDeathNotifications[id.listener];		DeathNotifications &deathNotifications = mDeathNotifications[bufferId->listener];
	deathNotifications.indices[id.frameIndex].emplace_back(id.bufferIndex);		deathNotifications.indices[bufferId->frameIndex].emplace_back(bufferId->bufferIndex);
	++deathNotifications.count;		++deathNotifications.count;
	mOnBufferDestroyed.notify_one();		mOnBufferDestroyed.notify_one();

			mTrackedBufferCache.erase(bufferId);
			delete bufferId;
	}		}

	// Notify the clients about buffer destructions.		// Notify the clients about buffer destructions.

media/codec2/hidl/1.0/utils/include/codec2/hidl/1.0/InputBufferManager.h

+12 −6

Original line number	Original line	Diff line number	Diff line
	@@ -196,13 +196,9 @@ private:
	frameIndex(frameIndex),		frameIndex(frameIndex),
	bufferIndex(bufferIndex),		bufferIndex(bufferIndex),
	buffer(buffer) {}		buffer(buffer) {}
	TrackedBuffer(const TrackedBuffer&) = default;
	bool operator<(const TrackedBuffer& other) const {
	return bufferIndex < other.bufferIndex;
	}
	};		};

	// Map: listener -> frameIndex -> set<TrackedBuffer>.		// Map: listener -> frameIndex -> set<TrackedBuffer*>.
	// Essentially, this is used to store triples (listener, frameIndex,		// Essentially, this is used to store triples (listener, frameIndex,
	// bufferIndex) that's searchable by listener and (listener, frameIndex).		// bufferIndex) that's searchable by listener and (listener, frameIndex).
	// However, the value of the innermost map is TrackedBuffer, which also		// However, the value of the innermost map is TrackedBuffer, which also
	@@ -210,7 +206,7 @@ private:
	// because onBufferDestroyed() needs to know listener and frameIndex too.		// because onBufferDestroyed() needs to know listener and frameIndex too.
	typedef std::map<wp<IComponentListener>,		typedef std::map<wp<IComponentListener>,
	std::map<uint64_t,		std::map<uint64_t,
	std::set<TrackedBuffer>>> TrackedBuffersMap;		std::set<TrackedBuffer*>>> TrackedBuffersMap;

	// Storage for pending (unsent) death notifications for one listener.		// Storage for pending (unsent) death notifications for one listener.
	// Each pair in member named "indices" are (frameIndex, bufferIndex) from		// Each pair in member named "indices" are (frameIndex, bufferIndex) from
	@@ -247,6 +243,16 @@ private:
	// Mutex for the management of all input buffers.		// Mutex for the management of all input buffers.
	std::mutex mMutex;		std::mutex mMutex;

			// Cache for all TrackedBuffers.
			//
			// Whenever registerOnDestroyNotify() is called, an argument of type
			// TrackedBuffer is created and stored into this cache.
			// Whenever unregisterOnDestroyNotify() or onBufferDestroyed() is called,
			// the TrackedBuffer is removed from this cache.
			//
			// mTrackedBuffersMap stores references to TrackedBuffers inside this cache.
			std::set<TrackedBuffer*> mTrackedBufferCache;

	// Tracked input buffers.		// Tracked input buffers.
	TrackedBuffersMap mTrackedBuffersMap;		TrackedBuffersMap mTrackedBuffersMap;