commit 15fff8c17da046c644334025c7707de81645c14d
author Yifan Hong <elsk@google.com> Tue Aug 10 15:07:56 2021 -0700
committer Yifan Hong <elsk@google.com> Thu Aug 12 11:56:35 2021 -0700
tree 251745ead7bb495661ee13e408c528b67900d46d
parent d17353cb5b20d50d41d28a189df9426a4120408f

binder: libbinder_tls ensure to check trigger at least once.

If the SSL_write / SSL_read finishes in one round, we might have
never checked the fdTrigger. Ensure to check it at least once
before SSL_write / SSL_read.

Test: binderRpcTest
Bug: 190868302
Change-Id: I4588475354025f8f02d3eb998e9390d5babbc860

libs/binder/FdTrigger.cpp

diff --git a/libs/binder/FdTrigger.cpp b/libs/binder/FdTrigger.cpp
index ecf13dc..b197a6a 100644
--- a/libs/binder/FdTrigger.cpp
+++ b/libs/binder/FdTrigger.cpp
@@ -59,4 +59,19 @@
     }
 }
 
+android::base::Result<bool> FdTrigger::isTriggeredPolled() {
+    pollfd pfd{.fd = mRead.get(), .events = 0, .revents = 0};
+    int ret = TEMP_FAILURE_RETRY(poll(&pfd, 1, 0));
+    if (ret < 0) {
+        return android::base::ErrnoError() << "FdTrigger::isTriggeredPolled: Error in poll()";
+    }
+    if (ret == 0) {
+        return false;
+    }
+    if (pfd.revents & POLLHUP) {
+        return true;
+    }
+    return android::base::Error() << "FdTrigger::isTriggeredPolled: poll() returns " << pfd.revents;
+}
+
 } // namespace android

libs/binder/FdTrigger.h

diff --git a/libs/binder/FdTrigger.h b/libs/binder/FdTrigger.h
index 984e685..a428417 100644
--- a/libs/binder/FdTrigger.h
+++ b/libs/binder/FdTrigger.h
@@ -16,6 +16,7 @@
 
 #include <memory>
 
+#include <android-base/result.h>
 #include <android-base/unique_fd.h>
 #include <utils/Errors.h>
 
@@ -34,7 +35,7 @@
     void trigger();
 
     /**
-     * Whether this has been triggered.
+     * Check whether this has been triggered by checking the write end.
      */
     bool isTriggered();
 
@@ -49,6 +50,16 @@
      */
     status_t triggerablePoll(base::borrowed_fd fd, int16_t event);
 
+    /**
+     * Check whether this has been triggered by poll()ing the read end.
+     *
+     * Return:
+     *   true - triggered
+     *   false - not triggered
+     *   error - error when polling
+     */
+    android::base::Result<bool> isTriggeredPolled();
+
 private:
     base::unique_fd mWrite;
     base::unique_fd mRead;

libs/binder/RpcTransportTls.cpp

diff --git a/libs/binder/RpcTransportTls.cpp b/libs/binder/RpcTransportTls.cpp
index 6d39890..a102913 100644
--- a/libs/binder/RpcTransportTls.cpp
+++ b/libs/binder/RpcTransportTls.cpp
@@ -304,6 +304,8 @@
 private:
     android::base::unique_fd mSocket;
     Ssl mSsl;
+
+    static status_t isTriggered(FdTrigger* fdTrigger);
 };
 
 // Error code is errno.
@@ -324,6 +326,15 @@
     return ret;
 }
 
+status_t RpcTransportTls::isTriggered(FdTrigger* fdTrigger) {
+    auto ret = fdTrigger->isTriggeredPolled();
+    if (!ret.ok()) {
+        ALOGE("%s: %s", __PRETTY_FUNCTION__, ret.error().message().c_str());
+        return ret.error().code() == 0 ? UNKNOWN_ERROR : -ret.error().code();
+    }
+    return OK;
+}
+
 status_t RpcTransportTls::interruptableWriteFully(FdTrigger* fdTrigger, const void* data,
                                                   size_t size) {
     auto buffer = reinterpret_cast<const uint8_t*>(data);
@@ -331,6 +342,10 @@
 
     MAYBE_WAIT_IN_FLAKE_MODE;
 
+    // Before doing any I/O, check trigger once. This ensures the trigger is checked at least
+    // once. The trigger is also checked via triggerablePoll() after every SSL_write().
+    if (status_t status = isTriggered(fdTrigger); status != OK) return status;
+
     while (buffer < end) {
         size_t todo = std::min<size_t>(end - buffer, std::numeric_limits<int>::max());
         auto [writeSize, errorQueue] = mSsl.call(SSL_write, buffer, todo);
@@ -358,6 +373,10 @@
 
     MAYBE_WAIT_IN_FLAKE_MODE;
 
+    // Before doing any I/O, check trigger once. This ensures the trigger is checked at least
+    // once. The trigger is also checked via triggerablePoll() after every SSL_write().
+    if (status_t status = isTriggered(fdTrigger); status != OK) return status;
+
     while (buffer < end) {
         size_t todo = std::min<size_t>(end - buffer, std::numeric_limits<int>::max());
         auto [readSize, errorQueue] = mSsl.call(SSL_read, buffer, todo);