Merge "Fix deadlock that can occur when BBQ's destructor runs while the BQ lock is held" into main (78d45e8f) · Commits · e / os / android_frameworks_native

libs/gui/BLASTBufferQueue.cpp

+82 −35

Original line number	Original line	Diff line number	Diff line
	@@ -227,11 +227,6 @@ void BLASTBufferQueue::initialize() {
	},		},
	this);		this);

	#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
	gui::BufferReleaseChannel::open(mName, mBufferReleaseConsumer, mBufferReleaseProducer);
	mBufferReleaseReader.emplace(*this);
	#endif

	// safe default, most producers are expected to override this		// safe default, most producers are expected to override this
	mProducer->setMaxDequeuedBufferCount(2);		mProducer->setMaxDequeuedBufferCount(2);

	@@ -1163,20 +1158,50 @@ public:
	};		};

	#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)		#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)

			// BufferReleaseReader is used to do blocking but interruptible reads from the buffer
			// release channel. To implement this, BufferReleaseReader owns an epoll file descriptor that
			// is configured to wake up when either the BufferReleaseReader::ConsumerEndpoint or an eventfd
			// becomes readable. Interrupts are necessary because a free buffer may become available for
			// reasons other than a buffer release from the producer.
			class BufferReleaseReader {
			public:
			explicit BufferReleaseReader(std::unique_ptr<gui::BufferReleaseChannel::ConsumerEndpoint>);

			BufferReleaseReader(const BufferReleaseReader&) = delete;
			BufferReleaseReader& operator=(const BufferReleaseReader&) = delete;

			// Block until we can read a buffer release message.
			//
			// Returns:
			// * OK if a ReleaseCallbackId and Fence were successfully read.
			// * WOULD_BLOCK if the blocking read was interrupted by interruptBlockingRead.
			// * TIMED_OUT if the blocking read timed out.
			// * UNKNOWN_ERROR if something went wrong.
			status_t readBlocking(ReleaseCallbackId& outId, sp<Fence>& outReleaseFence,
			uint32_t& outMaxAcquiredBufferCount, nsecs_t timeout);

			status_t readNonBlocking(ReleaseCallbackId& outId, sp<Fence>& outReleaseFence,
			uint32_t& outMaxAcquiredBufferCount);

			void interruptBlockingRead();
			void clearInterrupts();

			private:
			std::unique_ptr<gui::BufferReleaseChannel::ConsumerEndpoint> mConsumerEndpoint;
			android::base::unique_fd mEpollFd;
			android::base::unique_fd mEventFd;
			};

	class BBQBufferQueueCore : public BufferQueueCore {		class BBQBufferQueueCore : public BufferQueueCore {
	public:		public:
	explicit BBQBufferQueueCore(const wp<BLASTBufferQueue>& bbq) : mBLASTBufferQueue{bbq} {}		explicit BBQBufferQueueCore(std::shared_ptr<BufferReleaseReader> bufferReleaseReader)
			: mBufferReleaseReader{std::move(bufferReleaseReader)} {}

	void notifyBufferReleased() const override {		void notifyBufferReleased() const override { mBufferReleaseReader->interruptBlockingRead(); }
	sp<BLASTBufferQueue> bbq = mBLASTBufferQueue.promote();
	if (!bbq) {
	return;
	}
	bbq->mBufferReleaseReader->interruptBlockingRead();
	}

	private:		private:
	wp<BLASTBufferQueue> mBLASTBufferQueue;		std::shared_ptr<BufferReleaseReader> mBufferReleaseReader;
	};		};
	#endif		#endif

	@@ -1184,9 +1209,17 @@ private:
	// can be non-blocking when the producer is in the client process.		// can be non-blocking when the producer is in the client process.
	class BBQBufferQueueProducer : public BufferQueueProducer {		class BBQBufferQueueProducer : public BufferQueueProducer {
	public:		public:
			#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
			BBQBufferQueueProducer(const sp<BufferQueueCore>& core, const wp<BLASTBufferQueue>& bbq,
			std::shared_ptr<BufferReleaseReader> bufferReleaseReader)
			: BufferQueueProducer(core, false /* consumerIsSurfaceFlinger*/),
			mBLASTBufferQueue(bbq),
			mBufferReleaseReader(std::move(bufferReleaseReader)) {}
			#else
	BBQBufferQueueProducer(const sp<BufferQueueCore>& core, const wp<BLASTBufferQueue>& bbq)		BBQBufferQueueProducer(const sp<BufferQueueCore>& core, const wp<BLASTBufferQueue>& bbq)
	: BufferQueueProducer(core, false /* consumerIsSurfaceFlinger*/),		: BufferQueueProducer(core, false /* consumerIsSurfaceFlinger*/),
	mBLASTBufferQueue(bbq) {}		mBLASTBufferQueue(bbq) {}
			#endif

	status_t connect(const sp<IProducerListener>& listener, int api, bool producerControlledByApp,		status_t connect(const sp<IProducerListener>& listener, int api, bool producerControlledByApp,
	QueueBufferOutput* output) override {		QueueBufferOutput* output) override {
	@@ -1234,24 +1267,20 @@ public:
	#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)		#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
	status_t waitForBufferRelease(std::unique_lock<std::mutex>& bufferQueueLock,		status_t waitForBufferRelease(std::unique_lock<std::mutex>& bufferQueueLock,
	nsecs_t timeout) const override {		nsecs_t timeout) const override {
			ATRACE_CALL();
	const auto startTime = std::chrono::steady_clock::now();		const auto startTime = std::chrono::steady_clock::now();
	sp<BLASTBufferQueue> bbq = mBLASTBufferQueue.promote();
	if (!bbq) {
	return OK;
	}

	// BufferQueue has already checked if we have a free buffer. If there's an unread interrupt,		// BufferQueue has already checked if we have a free buffer. If there's an unread interrupt,
	// we want to ignore it. This must be done before unlocking the BufferQueue lock to ensure		// we want to ignore it. This must be done before unlocking the BufferQueue lock to ensure
	// we don't miss an interrupt.		// we don't miss an interrupt.
	bbq->mBufferReleaseReader->clearInterrupts();		mBufferReleaseReader->clearInterrupts();
	UnlockGuard unlockGuard{bufferQueueLock};		UnlockGuard unlockGuard{bufferQueueLock};

	ATRACE_FORMAT("waiting for free buffer");
	ReleaseCallbackId id;		ReleaseCallbackId id;
	sp<Fence> fence;		sp<Fence> fence;
	uint32_t maxAcquiredBufferCount;		uint32_t maxAcquiredBufferCount;
	status_t status =		status_t status =
	bbq->mBufferReleaseReader->readBlocking(id, fence, maxAcquiredBufferCount, timeout);		mBufferReleaseReader->readBlocking(id, fence, maxAcquiredBufferCount, timeout);
	if (status == TIMED_OUT) {		if (status == TIMED_OUT) {
	return TIMED_OUT;		return TIMED_OUT;
	} else if (status != OK) {		} else if (status != OK) {
	@@ -1260,6 +1289,11 @@ public:
	return OK;		return OK;
	}		}

			sp<BLASTBufferQueue> bbq = mBLASTBufferQueue.promote();
			if (!bbq) {
			return OK;
			}

	bbq->releaseBufferCallback(id, fence, maxAcquiredBufferCount);		bbq->releaseBufferCallback(id, fence, maxAcquiredBufferCount);
	const nsecs_t durationNanos = std::chrono::duration_cast<std::chrono::nanoseconds>(		const nsecs_t durationNanos = std::chrono::duration_cast<std::chrono::nanoseconds>(
	std::chrono::steady_clock::now() - startTime)		std::chrono::steady_clock::now() - startTime)
	@@ -1275,6 +1309,7 @@ public:

	private:		private:
	const wp<BLASTBufferQueue> mBLASTBufferQueue;		const wp<BLASTBufferQueue> mBLASTBufferQueue;
			std::shared_ptr<BufferReleaseReader> mBufferReleaseReader;
	};		};

	// Similar to BufferQueue::createBufferQueue but creates an adapter specific bufferqueue producer.		// Similar to BufferQueue::createBufferQueue but creates an adapter specific bufferqueue producer.
	@@ -1287,14 +1322,23 @@ void BLASTBufferQueue::createBufferQueue(sp<IGraphicBufferProducer>* outProducer
	LOG_ALWAYS_FATAL_IF(outConsumer == nullptr, "BLASTBufferQueue: outConsumer must not be NULL");		LOG_ALWAYS_FATAL_IF(outConsumer == nullptr, "BLASTBufferQueue: outConsumer must not be NULL");

	#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)		#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
	auto core = sp<BBQBufferQueueCore>::make(wp<BLASTBufferQueue>::fromExisting(this));		std::unique_ptr<gui::BufferReleaseChannel::ConsumerEndpoint> bufferReleaseConsumer;
			gui::BufferReleaseChannel::open(mName, bufferReleaseConsumer, mBufferReleaseProducer);
			mBufferReleaseReader = std::make_shared<BufferReleaseReader>(std::move(bufferReleaseConsumer));

			auto core = sp<BBQBufferQueueCore>::make(mBufferReleaseReader);
	#else		#else
	auto core = sp<BufferQueueCore>::make();		auto core = sp<BufferQueueCore>::make();
	#endif		#endif
	LOG_ALWAYS_FATAL_IF(core == nullptr, "BLASTBufferQueue: failed to create BufferQueueCore");		LOG_ALWAYS_FATAL_IF(core == nullptr, "BLASTBufferQueue: failed to create BufferQueueCore");

			#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
			auto producer = sp<BBQBufferQueueProducer>::make(core, wp<BLASTBufferQueue>::fromExisting(this),
			mBufferReleaseReader);
			#else
	auto producer =		auto producer =
	sp<BBQBufferQueueProducer>::make(core, wp<BLASTBufferQueue>::fromExisting(this));		sp<BBQBufferQueueProducer>::make(core, wp<BLASTBufferQueue>::fromExisting(this));
			#endif
	LOG_ALWAYS_FATAL_IF(producer == nullptr,		LOG_ALWAYS_FATAL_IF(producer == nullptr,
	"BLASTBufferQueue: failed to create BBQBufferQueueProducer");		"BLASTBufferQueue: failed to create BBQBufferQueueProducer");

	@@ -1394,8 +1438,7 @@ void BLASTBufferQueue::drainBufferReleaseConsumer() {
	ReleaseCallbackId id;		ReleaseCallbackId id;
	sp<Fence> fence;		sp<Fence> fence;
	uint32_t maxAcquiredBufferCount;		uint32_t maxAcquiredBufferCount;
	status_t status =		status_t status = mBufferReleaseReader->readNonBlocking(id, fence, maxAcquiredBufferCount);
	mBufferReleaseConsumer->readReleaseFence(id, fence, maxAcquiredBufferCount);
	if (status != OK) {		if (status != OK) {
	return;		return;
	}		}
	@@ -1403,7 +1446,9 @@ void BLASTBufferQueue::drainBufferReleaseConsumer() {
	}		}
	}		}

	BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader(BLASTBufferQueue& bbq) : mBbq{bbq} {		BufferReleaseReader::BufferReleaseReader(
			std::unique_ptr<gui::BufferReleaseChannel::ConsumerEndpoint> consumerEndpoint)
			: mConsumerEndpoint{std::move(consumerEndpoint)} {
	mEpollFd = android::base::unique_fd{epoll_create1(EPOLL_CLOEXEC)};		mEpollFd = android::base::unique_fd{epoll_create1(EPOLL_CLOEXEC)};
	LOG_ALWAYS_FATAL_IF(!mEpollFd.ok(),		LOG_ALWAYS_FATAL_IF(!mEpollFd.ok(),
	"Failed to create buffer release epoll file descriptor. errno=%d "		"Failed to create buffer release epoll file descriptor. errno=%d "
	@@ -1412,8 +1457,8 @@ BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader(BLASTBufferQueue& bbq

	epoll_event registerEndpointFd{};		epoll_event registerEndpointFd{};
	registerEndpointFd.events = EPOLLIN;		registerEndpointFd.events = EPOLLIN;
	registerEndpointFd.data.fd = mBbq.mBufferReleaseConsumer->getFd();		registerEndpointFd.data.fd = mConsumerEndpoint->getFd();
	status_t status = epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mBbq.mBufferReleaseConsumer->getFd(),		status_t status = epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mConsumerEndpoint->getFd(),
	&registerEndpointFd);		&registerEndpointFd);
	LOG_ALWAYS_FATAL_IF(status == -1,		LOG_ALWAYS_FATAL_IF(status == -1,
	"Failed to register buffer release consumer file descriptor with epoll. "		"Failed to register buffer release consumer file descriptor with epoll. "
	@@ -1436,10 +1481,8 @@ BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader(BLASTBufferQueue& bbq
	errno, strerror(errno));		errno, strerror(errno));
	}		}

	status_t BLASTBufferQueue::BufferReleaseReader::readBlocking(ReleaseCallbackId& outId,		status_t BufferReleaseReader::readBlocking(ReleaseCallbackId& outId, sp<Fence>& outFence,
	sp<Fence>& outFence,		uint32_t& outMaxAcquiredBufferCount, nsecs_t timeout) {
	uint32_t& outMaxAcquiredBufferCount,
	nsecs_t timeout) {
	// TODO(b/363290953) epoll_wait only has millisecond timeout precision. If timeout is less than		// TODO(b/363290953) epoll_wait only has millisecond timeout precision. If timeout is less than
	// 1ms, then we round timeout up to 1ms. Otherwise, we round timeout to the nearest		// 1ms, then we round timeout up to 1ms. Otherwise, we round timeout to the nearest
	// millisecond. Once epoll_pwait2 can be used in libgui, we can specify timeout with nanosecond		// millisecond. Once epoll_pwait2 can be used in libgui, we can specify timeout with nanosecond
	@@ -1479,17 +1522,21 @@ status_t BLASTBufferQueue::BufferReleaseReader::readBlocking(ReleaseCallbackId&
	return WOULD_BLOCK;		return WOULD_BLOCK;
	}		}

	return mBbq.mBufferReleaseConsumer->readReleaseFence(outId, outFence,		return mConsumerEndpoint->readReleaseFence(outId, outFence, outMaxAcquiredBufferCount);
	outMaxAcquiredBufferCount);		}

			status_t BufferReleaseReader::readNonBlocking(ReleaseCallbackId& outId, sp<Fence>& outFence,
			uint32_t& outMaxAcquiredBufferCount) {
			return mConsumerEndpoint->readReleaseFence(outId, outFence, outMaxAcquiredBufferCount);
	}		}

	void BLASTBufferQueue::BufferReleaseReader::interruptBlockingRead() {		void BufferReleaseReader::interruptBlockingRead() {
	if (eventfd_write(mEventFd.get(), 1) == -1) {		if (eventfd_write(mEventFd.get(), 1) == -1) {
	ALOGE("failed to notify dequeue event. errno=%d message='%s'", errno, strerror(errno));		ALOGE("failed to notify dequeue event. errno=%d message='%s'", errno, strerror(errno));
	}		}
	}		}

	void BLASTBufferQueue::BufferReleaseReader::clearInterrupts() {		void BufferReleaseReader::clearInterrupts() {
	eventfd_t value;		eventfd_t value;
	if (eventfd_read(mEventFd.get(), &value) == -1 && errno != EWOULDBLOCK) {		if (eventfd_read(mEventFd.get(), &value) == -1 && errno != EWOULDBLOCK) {
	ALOGE("error while reading from eventfd. errno=%d message='%s'", errno, strerror(errno));		ALOGE("error while reading from eventfd. errno=%d message='%s'", errno, strerror(errno));

libs/gui/include/gui/BLASTBufferQueue.h

+2 −34

Original line number	Original line	Diff line number	Diff line
	@@ -43,6 +43,7 @@ constexpr size_t kDequeueTimestampsMapSizeHint = 32;

	class BLASTBufferQueue;		class BLASTBufferQueue;
	class BufferItemConsumer;		class BufferItemConsumer;
			class BufferReleaseReader;

	class BLASTBufferItemConsumer : public BufferItemConsumer {		class BLASTBufferItemConsumer : public BufferItemConsumer {
	public:		public:
	@@ -345,45 +346,12 @@ private:
	#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)		#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
	// BufferReleaseChannel is used to communicate buffer releases from SurfaceFlinger to the		// BufferReleaseChannel is used to communicate buffer releases from SurfaceFlinger to the
	// client.		// client.
	std::unique_ptr<gui::BufferReleaseChannel::ConsumerEndpoint> mBufferReleaseConsumer;
	std::shared_ptr<gui::BufferReleaseChannel::ProducerEndpoint> mBufferReleaseProducer;		std::shared_ptr<gui::BufferReleaseChannel::ProducerEndpoint> mBufferReleaseProducer;

	void updateBufferReleaseProducer() REQUIRES(mMutex);		void updateBufferReleaseProducer() REQUIRES(mMutex);
	void drainBufferReleaseConsumer();		void drainBufferReleaseConsumer();

	// BufferReleaseReader is used to do blocking but interruptible reads from the buffer		std::shared_ptr<BufferReleaseReader> mBufferReleaseReader;
	// release channel. To implement this, BufferReleaseReader owns an epoll file descriptor that
	// is configured to wake up when either the BufferReleaseReader::ConsumerEndpoint or an eventfd
	// becomes readable. Interrupts are necessary because a free buffer may become available for
	// reasons other than a buffer release from the producer.
	class BufferReleaseReader {
	public:
	explicit BufferReleaseReader(BLASTBufferQueue&);

	BufferReleaseReader(const BufferReleaseReader&) = delete;
	BufferReleaseReader& operator=(const BufferReleaseReader&) = delete;

	// Block until we can read a buffer release message.
	//
	// Returns:
	// * OK if a ReleaseCallbackId and Fence were successfully read.
	// * WOULD_BLOCK if the blocking read was interrupted by interruptBlockingRead.
	// * TIMED_OUT if the blocking read timed out.
	// * UNKNOWN_ERROR if something went wrong.
	status_t readBlocking(ReleaseCallbackId& outId, sp<Fence>& outReleaseFence,
	uint32_t& outMaxAcquiredBufferCount, nsecs_t timeout);

	void interruptBlockingRead();
	void clearInterrupts();

	private:
	BLASTBufferQueue& mBbq;

	android::base::unique_fd mEpollFd;
	android::base::unique_fd mEventFd;
	};

	std::optional<BufferReleaseReader> mBufferReleaseReader;
	#endif		#endif
	};		};