From b4eafd35890b1651514feb602aba025e4c869d8d Mon Sep 17 00:00:00 2001 From: Devin Moore Date: Mon, 18 Nov 2024 16:46:33 +0000 Subject: Add Accessor::from_binder This allows other processes to host the Accessor object from libbinder. This can be useful when another process has the sepolicy permissions to create the connection. Test: atest rustBinderTest Bug: 358427181 Change-Id: Iedc51e2fa5b50e91438690d5d551156927a91807 --- libs/binder/rust/src/system_only.rs | 26 ++++++++++++++++++++++++++ libs/binder/rust/tests/integration.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) (limited to 'libs') diff --git a/libs/binder/rust/src/system_only.rs b/libs/binder/rust/src/system_only.rs index 3da59ab811..1a58d6b44d 100644 --- a/libs/binder/rust/src/system_only.rs +++ b/libs/binder/rust/src/system_only.rs @@ -91,6 +91,32 @@ impl Accessor { Accessor { accessor } } + /// Creates a new Accessor instance based on an existing Accessor's binder. + /// This is useful when the Accessor instance is hosted in another process + /// that has the permissions to create the socket connection FD. + /// + /// The `instance` argument must match the instance that the original Accessor + /// is responsible for. + /// `instance` must not contain null bytes and is used to create a CString to + /// pass through FFI. + /// The `binder` argument must be a valid binder from an Accessor + pub fn from_binder(instance: &str, binder: SpIBinder) -> Option { + let inst = CString::new(instance).unwrap(); + + // Safety: All `SpIBinder` objects (the `binder` argument) hold a valid pointer + // to an `AIBinder` that is guaranteed to remain valid for the lifetime of the + // SpIBinder. `ABinderRpc_Accessor_fromBinder` creates a new pointer to that binder + // that it is responsible for. + // The `inst` argument is a new CString that will copied by + // `ABinderRpc_Accessor_fromBinder` and not modified. + let accessor = + unsafe { sys::ABinderRpc_Accessor_fromBinder(inst.as_ptr(), binder.as_raw()) }; + if accessor.is_null() { + return None; + } + Some(Accessor { accessor }) + } + /// Get the underlying binder for this Accessor for when it needs to be either /// registered with service manager or sent to another process. pub fn as_binder(&self) -> Option { diff --git a/libs/binder/rust/tests/integration.rs b/libs/binder/rust/tests/integration.rs index 0e793e5761..da4f128e67 100644 --- a/libs/binder/rust/tests/integration.rs +++ b/libs/binder/rust/tests/integration.rs @@ -1038,6 +1038,34 @@ mod tests { assert!(deleted.load(Ordering::Relaxed)); } + #[test] + fn test_accessor_from_accessor_binder() { + let get_connection_info = move |_instance: &str| None; + let accessor = Accessor::new("foo.service", get_connection_info); + let accessor2 = + Accessor::from_binder("foo.service", accessor.as_binder().unwrap()).unwrap(); + assert_eq!(accessor.as_binder(), accessor2.as_binder()); + } + + #[test] + fn test_accessor_from_non_accessor_binder() { + let service_name = "rust_test_ibinder"; + let _process = ScopedServiceProcess::new(service_name); + let binder = binder::get_service(service_name).unwrap(); + assert!(binder.is_binder_alive()); + + let accessor = Accessor::from_binder("rust_test_ibinder", binder); + assert!(accessor.is_none()); + } + + #[test] + fn test_accessor_from_wrong_accessor_binder() { + let get_connection_info = move |_instance: &str| None; + let accessor = Accessor::new("foo.service", get_connection_info); + let accessor2 = Accessor::from_binder("NOT.foo.service", accessor.as_binder().unwrap()); + assert!(accessor2.is_none()); + } + #[tokio::test] async fn reassociate_rust_binder_async() { let service_name = "testing_service"; -- cgit v1.2.3-59-g8ed1b From eb63bbfd16ed9732be13d2ea6699edc6d7e4b545 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Wed, 13 Nov 2024 15:13:29 -0800 Subject: Do not crash when server channel has closed If the publisher has been destroyed (or, equivalently, if the server channel was closed), the consumer should not crash. Instead, we should allow consumer to exit gracefully. In this CL, we specifically check for DEAD_OBJECT and drain the queue, printing all of the events that will never be delivered to the publisher for useful debugging purposes. Bug: 305165753 Test: TEST=libinput_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST Flag: EXEMPT bugfix Change-Id: I00973ebb87e0f59bfd3c0f58edf7355b28988c15 --- libs/input/InputConsumerNoResampling.cpp | 15 +++++++ .../InputPublisherAndConsumerNoResampling_test.cpp | 48 +++++++++++++++------- 2 files changed, 48 insertions(+), 15 deletions(-) (limited to 'libs') diff --git a/libs/input/InputConsumerNoResampling.cpp b/libs/input/InputConsumerNoResampling.cpp index 2c0f77a814..cd8582182a 100644 --- a/libs/input/InputConsumerNoResampling.cpp +++ b/libs/input/InputConsumerNoResampling.cpp @@ -169,6 +169,12 @@ InputMessage createTimelineMessage(int32_t inputEventId, nsecs_t gpuCompletedTim msg.body.timeline.graphicsTimeline[GraphicsTimeline::PRESENT_TIME] = presentTime; return msg; } + +std::ostream& operator<<(std::ostream& out, const InputMessage& msg) { + out << ftl::enum_string(msg.header.type); + return out; +} + } // namespace // --- InputConsumerNoResampling --- @@ -272,6 +278,15 @@ void InputConsumerNoResampling::processOutboundEvents() { return; // try again later } + if (result == DEAD_OBJECT) { + // If there's no one to receive events in the channel, there's no point in sending them. + // Drop all outbound events. + LOG(INFO) << "Channel " << mChannel->getName() << " died. Dropping outbound event " + << outboundMsg; + mOutboundQueue.pop(); + setFdEvents(0); + continue; + } // Some other error. Give up LOG(FATAL) << "Failed to send outbound event on channel '" << mChannel->getName() << "'. status=" << statusToString(result) << "(" << result << ")"; diff --git a/libs/input/tests/InputPublisherAndConsumerNoResampling_test.cpp b/libs/input/tests/InputPublisherAndConsumerNoResampling_test.cpp index 39bb841c0a..1dadae98e4 100644 --- a/libs/input/tests/InputPublisherAndConsumerNoResampling_test.cpp +++ b/libs/input/tests/InputPublisherAndConsumerNoResampling_test.cpp @@ -319,6 +319,8 @@ protected: protected: // Interaction with the looper thread + void blockLooper(); + void unblockLooper(); enum class LooperMessage : int { CALL_PROBABLY_HAS_INPUT, CREATE_CONSUMER, @@ -389,6 +391,26 @@ private: }; }; +void InputPublisherAndConsumerNoResamplingTest::blockLooper() { + { + std::scoped_lock l(mLock); + mLooperMayProceed = false; + } + sendMessage(LooperMessage::BLOCK_LOOPER); + { + std::unique_lock l(mLock); + mNotifyLooperWaiting.wait(l, [this] { return mLooperIsBlocked; }); + } +} + +void InputPublisherAndConsumerNoResamplingTest::unblockLooper() { + { + std::scoped_lock l(mLock); + mLooperMayProceed = true; + } + mNotifyLooperMayProceed.notify_all(); +} + void InputPublisherAndConsumerNoResamplingTest::sendMessage(LooperMessage message) { Message msg{ftl::to_underlying(message)}; mLooper->sendMessage(mMessageHandler, msg); @@ -600,15 +622,7 @@ void InputPublisherAndConsumerNoResamplingTest::publishAndConsumeSinglePointerMu std::queue publishedSequenceNumbers; // Block Looper to increase the chance of batching events - { - std::scoped_lock l(mLock); - mLooperMayProceed = false; - } - sendMessage(LooperMessage::BLOCK_LOOPER); - { - std::unique_lock l(mLock); - mNotifyLooperWaiting.wait(l, [this] { return mLooperIsBlocked; }); - } + blockLooper(); uint32_t firstSampleId; for (size_t i = 0; i < nSamples; ++i) { @@ -629,12 +643,7 @@ void InputPublisherAndConsumerNoResamplingTest::publishAndConsumeSinglePointerMu std::vector singleSampledMotionEvents; - // Unblock Looper - { - std::scoped_lock l(mLock); - mLooperMayProceed = true; - } - mNotifyLooperMayProceed.notify_all(); + unblockLooper(); // We have no control over the socket behavior, so the consumer can receive // the motion as a batched event, or as a sequence of multiple single-sample MotionEvents (or a @@ -809,6 +818,15 @@ void InputPublisherAndConsumerNoResamplingTest::publishAndConsumeTouchModeEvent( verifyFinishedSignal(*mPublisher, seq, publishTime); } +/** + * If the publisher has died, consumer should not crash when trying to send an outgoing message. + */ +TEST_F(InputPublisherAndConsumerNoResamplingTest, ConsumerWritesAfterPublisherDies) { + mPublisher.reset(); // The publisher has died + mReportTimelineArgs.emplace(/*inputEventId=*/10, /*gpuCompletedTime=*/20, /*presentTime=*/30); + sendMessage(LooperMessage::CALL_REPORT_TIMELINE); +} + TEST_F(InputPublisherAndConsumerNoResamplingTest, SendTimeline) { const int32_t inputEventId = 20; const nsecs_t gpuCompletedTime = 30; -- cgit v1.2.3-59-g8ed1b