Merge "Fix BufferHub state machine to return errors on invalid transitions." into oc-dr1-dev (2b7b5c34) · Commits · e / os / android_frameworks_native-old

libs/vr/libbufferhub/bufferhub_tests.cpp

+51 −0

Original line number	Diff line number	Diff line
		@@ -62,6 +62,57 @@ TEST_F(LibBufferHubTest, TestBasicUsage) {
		EXPECT_GE(0, RETRY_EINTR(p->Poll(0)));
		}

		TEST_F(LibBufferHubTest, TestStateTransitions) {
		std::unique_ptr<BufferProducer> p = BufferProducer::Create(
		kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t));
		ASSERT_TRUE(p.get() != nullptr);
		std::unique_ptr<BufferConsumer> c =
		BufferConsumer::Import(p->CreateConsumer());
		ASSERT_TRUE(c.get() != nullptr);

		uint64_t context;
		LocalHandle fence;

		// The producer buffer starts in gained state.

		// Acquire, release, and gain in gained state should fail.
		EXPECT_EQ(-EBUSY, c->Acquire(&fence, &context));
		EXPECT_EQ(-EBUSY, c->Release(LocalHandle()));
		EXPECT_EQ(-EALREADY, p->Gain(&fence));

		// Post in gained state should succeed.
		EXPECT_EQ(0, p->Post(LocalHandle(), kContext));

		// Post, release, and gain in posted state should fail.
		EXPECT_EQ(-EBUSY, p->Post(LocalHandle(), kContext));
		EXPECT_EQ(-EBUSY, c->Release(LocalHandle()));
		EXPECT_EQ(-EBUSY, p->Gain(&fence));

		// Acquire in posted state should succeed.
		EXPECT_LE(0, c->Acquire(&fence, &context));

		// Acquire, post, and gain in acquired state should fail.
		EXPECT_EQ(-EBUSY, c->Acquire(&fence, &context));
		EXPECT_EQ(-EBUSY, p->Post(LocalHandle(), kContext));
		EXPECT_EQ(-EBUSY, p->Gain(&fence));

		// Release in acquired state should succeed.
		EXPECT_EQ(0, c->Release(LocalHandle()));

		// Release, acquire, and post in released state should fail.
		EXPECT_EQ(-EBUSY, c->Release(LocalHandle()));
		EXPECT_EQ(-EBUSY, c->Acquire(&fence, &context));
		EXPECT_EQ(-EBUSY, p->Post(LocalHandle(), kContext));

		// Gain in released state should succeed.
		EXPECT_EQ(0, p->Gain(&fence));

		// Acquire, release, and gain in gained state should fail.
		EXPECT_EQ(-EBUSY, c->Acquire(&fence, &context));
		EXPECT_EQ(-EBUSY, c->Release(LocalHandle()));
		EXPECT_EQ(-EALREADY, p->Gain(&fence));
		}

		TEST_F(LibBufferHubTest, TestWithCustomMetadata) {
		struct Metadata {
		int64_t field1;

services/vr/bufferhubd/consumer_channel.cpp

+28 −18

Original line number	Diff line number	Diff line
		@@ -8,9 +8,9 @@
		#include <private/dvr/bufferhub_rpc.h>
		#include "producer_channel.h"

		using android::pdx::ErrorStatus;
		using android::pdx::BorrowedHandle;
		using android::pdx::Channel;
		using android::pdx::ErrorStatus;
		using android::pdx::Message;
		using android::pdx::Status;
		using android::pdx::rpc::DispatchRemoteMethod;
		@@ -22,8 +22,6 @@ ConsumerChannel::ConsumerChannel(BufferHubService* service, int buffer_id,
		int channel_id,
		const std::shared_ptr<Channel> producer)
		: BufferHubChannel(service, buffer_id, channel_id, kConsumerType),
		handled_(true),
		ignored_(false),
		producer_(producer) {
		GetProducer()->AddConsumer(this);
		}
		@@ -34,7 +32,7 @@ ConsumerChannel::~ConsumerChannel() {
		channel_id(), buffer_id());

		if (auto producer = GetProducer()) {
		if (!handled_) // Producer is waiting for our Release.
		if (!released_) // Producer is waiting for our Release.
		producer->OnConsumerIgnored();
		producer->RemoveConsumer(this);
		}
		@@ -108,15 +106,20 @@ ConsumerChannel::OnConsumerAcquire(Message& message,
		if (!producer)
		return ErrorStatus(EPIPE);

		if (ignored_ \|\| handled_) {
		if (acquired_ \|\| released_) {
		ALOGE(
		"ConsumerChannel::OnConsumerAcquire: Acquire when not posted: "
		"ignored=%d handled=%d channel_id=%d buffer_id=%d",
		ignored_, handled_, message.GetChannelId(), producer->buffer_id());
		"ignored=%d acquired=%d released=%d channel_id=%d buffer_id=%d",
		ignored_, acquired_, released_, message.GetChannelId(),
		producer->buffer_id());
		return ErrorStatus(EBUSY);
		} else {
		auto status = producer->OnConsumerAcquire(message, metadata_size);
		if (status) {
		ClearAvailable();
		return producer->OnConsumerAcquire(message, metadata_size);
		acquired_ = true;
		}
		return status;
		}
		}

		@@ -127,17 +130,21 @@ Status<void> ConsumerChannel::OnConsumerRelease(Message& message,
		if (!producer)
		return ErrorStatus(EPIPE);

		if (ignored_ \|\| handled_) {
		if (!acquired_ \|\| released_) {
		ALOGE(
		"ConsumerChannel::OnConsumerRelease: Release when not acquired: "
		"ignored=%d handled=%d channel_id=%d buffer_id=%d",
		ignored_, handled_, message.GetChannelId(), producer->buffer_id());
		"ignored=%d acquired=%d released=%d channel_id=%d buffer_id=%d",
		ignored_, acquired_, released_, message.GetChannelId(),
		producer->buffer_id());
		return ErrorStatus(EBUSY);
		} else {
		ClearAvailable();
		auto status =
		producer->OnConsumerRelease(message, std::move(release_fence));
		handled_ = !!status;
		if (status) {
		ClearAvailable();
		acquired_ = false;
		released_ = true;
		}
		return status;
		}
		}
		@@ -149,12 +156,13 @@ Status<void> ConsumerChannel::OnConsumerSetIgnore(Message&, bool ignored) {
		return ErrorStatus(EPIPE);

		ignored_ = ignored;
		if (ignored_ && !handled_) {
		if (ignored_ && acquired_) {
		// Update the producer if ignore is set after the consumer acquires the
		// buffer.
		ClearAvailable();
		producer->OnConsumerIgnored();
		handled_ = false;
		acquired_ = false;
		released_ = true;
		}

		return {};
		@@ -162,10 +170,12 @@ Status<void> ConsumerChannel::OnConsumerSetIgnore(Message&, bool ignored) {

		bool ConsumerChannel::OnProducerPosted() {
		if (ignored_) {
		handled_ = true;
		acquired_ = false;
		released_ = true;
		return false;
		} else {
		handled_ = false;
		acquired_ = false;
		released_ = false;
		SignalAvailable();
		return true;
		}

services/vr/bufferhubd/consumer_channel.h

+3 −2

Original line number	Diff line number	Diff line
		@@ -38,8 +38,9 @@ class ConsumerChannel : public BufferHubChannel {
		LocalFence release_fence);
		pdx::Status<void> OnConsumerSetIgnore(Message& message, bool ignore);

		bool handled_; // True if we have processed RELEASE.
		bool ignored_; // True if we are ignoring events.
		bool acquired_{false};
		bool released_{true};
		bool ignored_{false}; // True if we are ignoring events.
		std::weak_ptr<Channel> producer_;

		ConsumerChannel(const ConsumerChannel&) = delete;

services/vr/bufferhubd/producer_channel.cpp

+3 −3

Original line number	Diff line number	Diff line
		@@ -122,7 +122,7 @@ bool ProducerChannel::HandleMessage(Message& message) {
		}

		Status<NativeBufferHandle<BorrowedHandle>> ProducerChannel::OnGetBuffer(
		Message& message) {
		Message& /message/) {
		ATRACE_NAME("ProducerChannel::OnGetBuffer");
		ALOGD_IF(TRACE, "ProducerChannel::OnGetBuffer: buffer=%d", buffer_id());
		return {NativeBufferHandle<BorrowedHandle>(buffer_, buffer_id())};
		@@ -204,7 +204,7 @@ Status<void> ProducerChannel::OnProducerPost(
		return {};
		}

		Status<LocalFence> ProducerChannel::OnProducerGain(Message& message) {
		Status<LocalFence> ProducerChannel::OnProducerGain(Message& /message/) {
		ATRACE_NAME("ProducerChannel::OnGain");
		ALOGD_IF(TRACE, "ProducerChannel::OnGain: buffer_id=%d", buffer_id());
		if (producer_owns_) {
		@@ -224,7 +224,7 @@ Status<LocalFence> ProducerChannel::OnProducerGain(Message& message) {
		}

		Status<std::pair<BorrowedFence, BufferWrapper<std::uint8_t*>>>
		ProducerChannel::OnConsumerAcquire(Message& message,
		ProducerChannel::OnConsumerAcquire(Message& /message/,
		std::size_t metadata_size) {
		ATRACE_NAME("ProducerChannel::OnConsumerAcquire");
		ALOGD_IF(TRACE, "ProducerChannel::OnConsumerAcquire: buffer_id=%d",