Skip to content

Commit eb63bbf

Browse files
committed
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
1 parent cdde6f5 commit eb63bbf

2 files changed

Lines changed: 48 additions & 15 deletions

File tree

libs/input/InputConsumerNoResampling.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,12 @@ InputMessage createTimelineMessage(int32_t inputEventId, nsecs_t gpuCompletedTim
169169
msg.body.timeline.graphicsTimeline[GraphicsTimeline::PRESENT_TIME] = presentTime;
170170
return msg;
171171
}
172+
173+
std::ostream& operator<<(std::ostream& out, const InputMessage& msg) {
174+
out << ftl::enum_string(msg.header.type);
175+
return out;
176+
}
177+
172178
} // namespace
173179

174180
// --- InputConsumerNoResampling ---
@@ -272,6 +278,15 @@ void InputConsumerNoResampling::processOutboundEvents() {
272278
return; // try again later
273279
}
274280

281+
if (result == DEAD_OBJECT) {
282+
// If there's no one to receive events in the channel, there's no point in sending them.
283+
// Drop all outbound events.
284+
LOG(INFO) << "Channel " << mChannel->getName() << " died. Dropping outbound event "
285+
<< outboundMsg;
286+
mOutboundQueue.pop();
287+
setFdEvents(0);
288+
continue;
289+
}
275290
// Some other error. Give up
276291
LOG(FATAL) << "Failed to send outbound event on channel '" << mChannel->getName()
277292
<< "'. status=" << statusToString(result) << "(" << result << ")";

libs/input/tests/InputPublisherAndConsumerNoResampling_test.cpp

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,8 @@ class InputPublisherAndConsumerNoResamplingTest : public testing::Test,
319319

320320
protected:
321321
// Interaction with the looper thread
322+
void blockLooper();
323+
void unblockLooper();
322324
enum class LooperMessage : int {
323325
CALL_PROBABLY_HAS_INPUT,
324326
CREATE_CONSUMER,
@@ -389,6 +391,26 @@ class InputPublisherAndConsumerNoResamplingTest : public testing::Test,
389391
};
390392
};
391393

394+
void InputPublisherAndConsumerNoResamplingTest::blockLooper() {
395+
{
396+
std::scoped_lock l(mLock);
397+
mLooperMayProceed = false;
398+
}
399+
sendMessage(LooperMessage::BLOCK_LOOPER);
400+
{
401+
std::unique_lock l(mLock);
402+
mNotifyLooperWaiting.wait(l, [this] { return mLooperIsBlocked; });
403+
}
404+
}
405+
406+
void InputPublisherAndConsumerNoResamplingTest::unblockLooper() {
407+
{
408+
std::scoped_lock l(mLock);
409+
mLooperMayProceed = true;
410+
}
411+
mNotifyLooperMayProceed.notify_all();
412+
}
413+
392414
void InputPublisherAndConsumerNoResamplingTest::sendMessage(LooperMessage message) {
393415
Message msg{ftl::to_underlying(message)};
394416
mLooper->sendMessage(mMessageHandler, msg);
@@ -600,15 +622,7 @@ void InputPublisherAndConsumerNoResamplingTest::publishAndConsumeSinglePointerMu
600622
std::queue<uint32_t> publishedSequenceNumbers;
601623

602624
// Block Looper to increase the chance of batching events
603-
{
604-
std::scoped_lock l(mLock);
605-
mLooperMayProceed = false;
606-
}
607-
sendMessage(LooperMessage::BLOCK_LOOPER);
608-
{
609-
std::unique_lock l(mLock);
610-
mNotifyLooperWaiting.wait(l, [this] { return mLooperIsBlocked; });
611-
}
625+
blockLooper();
612626

613627
uint32_t firstSampleId;
614628
for (size_t i = 0; i < nSamples; ++i) {
@@ -629,12 +643,7 @@ void InputPublisherAndConsumerNoResamplingTest::publishAndConsumeSinglePointerMu
629643

630644
std::vector<MotionEvent> singleSampledMotionEvents;
631645

632-
// Unblock Looper
633-
{
634-
std::scoped_lock l(mLock);
635-
mLooperMayProceed = true;
636-
}
637-
mNotifyLooperMayProceed.notify_all();
646+
unblockLooper();
638647

639648
// We have no control over the socket behavior, so the consumer can receive
640649
// the motion as a batched event, or as a sequence of multiple single-sample MotionEvents (or a
@@ -809,6 +818,15 @@ void InputPublisherAndConsumerNoResamplingTest::publishAndConsumeTouchModeEvent(
809818
verifyFinishedSignal(*mPublisher, seq, publishTime);
810819
}
811820

821+
/**
822+
* If the publisher has died, consumer should not crash when trying to send an outgoing message.
823+
*/
824+
TEST_F(InputPublisherAndConsumerNoResamplingTest, ConsumerWritesAfterPublisherDies) {
825+
mPublisher.reset(); // The publisher has died
826+
mReportTimelineArgs.emplace(/*inputEventId=*/10, /*gpuCompletedTime=*/20, /*presentTime=*/30);
827+
sendMessage(LooperMessage::CALL_REPORT_TIMELINE);
828+
}
829+
812830
TEST_F(InputPublisherAndConsumerNoResamplingTest, SendTimeline) {
813831
const int32_t inputEventId = 20;
814832
const nsecs_t gpuCompletedTime = 30;

0 commit comments

Comments
 (0)