Merge changes I3a4f683f,I15de71ee,I93a27842,I4ad4d980 am: 99148c3722 am: ccbc7c5827
Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/1826700
Change-Id: I1be8c0b2252c65ca8d19a389cf9e104ef7abc6f7
diff --git a/libs/binder/FdTrigger.cpp b/libs/binder/FdTrigger.cpp
index b197a6a..ecf13dc 100644
--- a/libs/binder/FdTrigger.cpp
+++ b/libs/binder/FdTrigger.cpp
@@ -59,19 +59,4 @@
}
}
-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
diff --git a/libs/binder/FdTrigger.h b/libs/binder/FdTrigger.h
index a428417..a545d6c 100644
--- a/libs/binder/FdTrigger.h
+++ b/libs/binder/FdTrigger.h
@@ -35,9 +35,13 @@
void trigger();
/**
- * Check whether this has been triggered by checking the write end.
+ * Check whether this has been triggered by checking the write end. Note:
+ * this has no internal locking, and it is inherently racey, but this is
+ * okay, because if we accidentally return false when a trigger has already
+ * happened, we can imagine that instead, the scheduler actually executed
+ * the code which is polling isTriggered earlier.
*/
- bool isTriggered();
+ [[nodiscard]] bool isTriggered();
/**
* Poll for a read event.
@@ -48,17 +52,7 @@
* true - time to read!
* false - trigger happened
*/
- 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();
+ [[nodiscard]] status_t triggerablePoll(base::borrowed_fd fd, int16_t event);
private:
base::unique_fd mWrite;
diff --git a/libs/binder/RpcTransportRaw.cpp b/libs/binder/RpcTransportRaw.cpp
index c012df8..41f4a9f 100644
--- a/libs/binder/RpcTransportRaw.cpp
+++ b/libs/binder/RpcTransportRaw.cpp
@@ -35,20 +35,6 @@
class RpcTransportRaw : public RpcTransport {
public:
explicit RpcTransportRaw(android::base::unique_fd socket) : mSocket(std::move(socket)) {}
- Result<size_t> send(const void* buf, size_t size) {
- ssize_t ret = TEMP_FAILURE_RETRY(::send(mSocket.get(), buf, size, MSG_NOSIGNAL));
- if (ret < 0) {
- return ErrnoError() << "send()";
- }
- return ret;
- }
- Result<size_t> recv(void* buf, size_t size) {
- ssize_t ret = TEMP_FAILURE_RETRY(::recv(mSocket.get(), buf, size, MSG_NOSIGNAL));
- if (ret < 0) {
- return ErrnoError() << "recv()";
- }
- return ret;
- }
Result<size_t> peek(void *buf, size_t size) override {
ssize_t ret = TEMP_FAILURE_RETRY(::recv(mSocket.get(), buf, size, MSG_PEEK));
if (ret < 0) {
@@ -65,15 +51,17 @@
status_t status;
while ((status = fdTrigger->triggerablePoll(mSocket.get(), POLLOUT)) == OK) {
- auto writeSize = this->send(buffer, end - buffer);
- if (!writeSize.ok()) {
- LOG_RPC_DETAIL("RpcTransport::send(): %s", writeSize.error().message().c_str());
- return writeSize.error().code() == 0 ? UNKNOWN_ERROR : -writeSize.error().code();
+ ssize_t writeSize =
+ TEMP_FAILURE_RETRY(::send(mSocket.get(), buffer, end - buffer, MSG_NOSIGNAL));
+ if (writeSize < 0) {
+ int savedErrno = errno;
+ LOG_RPC_DETAIL("RpcTransport send(): %s", strerror(savedErrno));
+ return -savedErrno;
}
- if (*writeSize == 0) return DEAD_OBJECT;
+ if (writeSize == 0) return DEAD_OBJECT;
- buffer += *writeSize;
+ buffer += writeSize;
if (buffer == end) return OK;
}
return status;
@@ -87,15 +75,17 @@
status_t status;
while ((status = fdTrigger->triggerablePoll(mSocket.get(), POLLIN)) == OK) {
- auto readSize = this->recv(buffer, end - buffer);
- if (!readSize.ok()) {
- LOG_RPC_DETAIL("RpcTransport::recv(): %s", readSize.error().message().c_str());
- return readSize.error().code() == 0 ? UNKNOWN_ERROR : -readSize.error().code();
+ ssize_t readSize =
+ TEMP_FAILURE_RETRY(::recv(mSocket.get(), buffer, end - buffer, MSG_NOSIGNAL));
+ if (readSize < 0) {
+ int savedErrno = errno;
+ LOG_RPC_DETAIL("RpcTransport recv(): %s", strerror(savedErrno));
+ return -savedErrno;
}
- if (*readSize == 0) return DEAD_OBJECT; // EOF
+ if (readSize == 0) return DEAD_OBJECT; // EOF
- buffer += *readSize;
+ buffer += readSize;
if (buffer == end) return OK;
}
return status;
diff --git a/libs/binder/RpcTransportTls.cpp b/libs/binder/RpcTransportTls.cpp
index 63f9339..23088ad 100644
--- a/libs/binder/RpcTransportTls.cpp
+++ b/libs/binder/RpcTransportTls.cpp
@@ -319,8 +319,6 @@
private:
android::base::unique_fd mSocket;
Ssl mSsl;
-
- static status_t isTriggered(FdTrigger* fdTrigger);
};
// Error code is errno.
@@ -341,15 +339,6 @@
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 *ret ? -ECANCELED : OK;
-}
-
status_t RpcTransportTls::interruptableWriteFully(FdTrigger* fdTrigger, const void* data,
size_t size) {
auto buffer = reinterpret_cast<const uint8_t*>(data);
@@ -359,7 +348,7 @@
// 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;
+ if (fdTrigger->isTriggered()) return -ECANCELED;
while (buffer < end) {
size_t todo = std::min<size_t>(end - buffer, std::numeric_limits<int>::max());
@@ -390,7 +379,7 @@
// 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;
+ if (fdTrigger->isTriggered()) return -ECANCELED;
while (buffer < end) {
size_t todo = std::min<size_t>(end - buffer, std::numeric_limits<int>::max());
diff --git a/libs/binder/include/binder/RpcTransport.h b/libs/binder/include/binder/RpcTransport.h
index 1c0bb18..4fe2324 100644
--- a/libs/binder/include/binder/RpcTransport.h
+++ b/libs/binder/include/binder/RpcTransport.h
@@ -38,7 +38,7 @@
virtual ~RpcTransport() = default;
// replacement of ::recv(MSG_PEEK). Error code may not be set if TLS is enabled.
- virtual android::base::Result<size_t> peek(void *buf, size_t size) = 0;
+ [[nodiscard]] virtual android::base::Result<size_t> peek(void *buf, size_t size) = 0;
/**
* Read (or write), but allow to be interrupted by a trigger.
@@ -47,9 +47,10 @@
* OK - succeeded in completely processing 'size'
* error - interrupted (failure or trigger)
*/
- virtual status_t interruptableWriteFully(FdTrigger *fdTrigger, const void *buf,
- size_t size) = 0;
- virtual status_t interruptableReadFully(FdTrigger *fdTrigger, void *buf, size_t size) = 0;
+ [[nodiscard]] virtual status_t interruptableWriteFully(FdTrigger *fdTrigger, const void *buf,
+ size_t size) = 0;
+ [[nodiscard]] virtual status_t interruptableReadFully(FdTrigger *fdTrigger, void *buf,
+ size_t size) = 0;
protected:
RpcTransport() = default;