From be41ae566617bc6e063011b1216a0aaa3e9ca80d Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 19 May 2020 20:41:36 -0700 Subject: [PATCH] adb: fix flakiness in PTY shell protocol. When a subprocess closes its PTY slave, the master fd will report POLLHUP when polled. This leads to us prematurely tearing everything down, without reading out output that's been written to the PTY. Resolve this by waiting until the fd no longer reports POLLIN. Bug: http://b/156551485 Bug: http://b/156552734 Test: `adb shell 'X=0; while /data/nativetest64/adbd_test/adbd_test --gtest_filter="ShellServiceTest.*Pty*" >/dev/null 2>&1; do X=$((X+1)); echo $X; done'` for 1000 iterations (failed within 20, previously) Test: test_device.py Change-Id: Ie591e0cafb532cd6cebdf6f356dc967565b5a2d9 --- adb/daemon/shell_service.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/adb/daemon/shell_service.cpp b/adb/daemon/shell_service.cpp index fbfae1e44..dbca4adb6 100644 --- a/adb/daemon/shell_service.cpp +++ b/adb/daemon/shell_service.cpp @@ -646,15 +646,21 @@ unique_fd* Subprocess::PollLoop(SubprocessPollfds* pfds) { } // After handling all of the events we've received, check to see if any fds have died. - if (stdinout_pfd.revents & (POLLHUP | POLLRDHUP | POLLERR | POLLNVAL)) { + auto poll_finished = [](int events) { + // Don't return failure until we've read out all of the fd's incoming data. + return (events & POLLIN) == 0 && + (events & (POLLHUP | POLLRDHUP | POLLERR | POLLNVAL)) != 0; + }; + + if (poll_finished(stdinout_pfd.revents)) { return &stdinout_sfd_; } - if (stderr_pfd.revents & (POLLHUP | POLLRDHUP | POLLERR | POLLNVAL)) { + if (poll_finished(stderr_pfd.revents)) { return &stderr_sfd_; } - if (protocol_pfd.revents & (POLLHUP | POLLRDHUP | POLLERR | POLLNVAL)) { + if (poll_finished(protocol_pfd.revents)) { return &protocol_sfd_; } } // while (!dead_sfd)