libc: __system_property_set uses writev to write atomically
__system_property_set sometimes produces broken_pipe error
when trying to write a property.
This change improves error messages and uses writev() instead
of sequence of send() calls.
Bug: http://b/35381074
Test: bionic-unit-tests --gtest_filter=prop*
Change-Id: I7a5b169c015db4e6b720370e58662de8206d1086
diff --git a/libc/bionic/system_properties.cpp b/libc/bionic/system_properties.cpp
index 0f2a7b5..ec1d18f 100644
--- a/libc/bionic/system_properties.cpp
+++ b/libc/bionic/system_properties.cpp
@@ -34,7 +34,6 @@
#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>
-#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
@@ -47,6 +46,7 @@
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/types.h>
+#include <sys/uio.h>
#include <sys/un.h>
#include <sys/xattr.h>
@@ -487,8 +487,8 @@
class PropertyServiceConnection {
public:
PropertyServiceConnection() : last_error_(0) {
- fd_ = socket(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0);
- if (fd_ == -1) {
+ socket_ = ::socket(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0);
+ if (socket_ == -1) {
last_error_ = errno;
return;
}
@@ -500,54 +500,33 @@
addr.sun_family = AF_LOCAL;
socklen_t alen = namelen + offsetof(sockaddr_un, sun_path) + 1;
- if (TEMP_FAILURE_RETRY(connect(fd_, reinterpret_cast<sockaddr*>(&addr), alen)) == -1) {
- close(fd_);
- fd_ = -1;
+ if (TEMP_FAILURE_RETRY(connect(socket_, reinterpret_cast<sockaddr*>(&addr), alen)) == -1) {
+ close(socket_);
+ socket_ = -1;
last_error_ = errno;
}
}
bool IsValid() {
- return fd_ != -1;
+ return socket_ != -1;
}
int GetLastError() {
return last_error_;
}
- bool SendUint32(uint32_t value) {
- int result = TEMP_FAILURE_RETRY(send(fd_, &value, sizeof(value), 0));
- return CheckSendRecvResult(result, sizeof(value));
- }
-
- bool SendString(const char* value) {
- uint32_t valuelen = strlen(value);
- if (!SendUint32(valuelen)) {
- return false;
- }
-
- // Trying to send even 0 bytes to closed socket may lead to
- // broken pipe (http://b/34670529).
- if (valuelen == 0) {
- return true;
- }
-
- int result = TEMP_FAILURE_RETRY(send(fd_, value, valuelen, 0));
- return CheckSendRecvResult(result, valuelen);
- }
-
bool RecvInt32(int32_t* value) {
- int result = TEMP_FAILURE_RETRY(recv(fd_, value, sizeof(*value), MSG_WAITALL));
+ int result = TEMP_FAILURE_RETRY(recv(socket_, value, sizeof(*value), MSG_WAITALL));
return CheckSendRecvResult(result, sizeof(*value));
}
- int GetFd() {
- return fd_;
+ int socket() {
+ return socket_;
}
~PropertyServiceConnection() {
- if (fd_ != -1) {
- close(fd_);
+ if (socket_ != -1) {
+ close(socket_);
}
}
@@ -564,8 +543,69 @@
return last_error_ == 0;
}
- int fd_;
+ int socket_;
int last_error_;
+
+ friend class SocketWriter;
+};
+
+class SocketWriter {
+ public:
+ explicit SocketWriter(PropertyServiceConnection* connection)
+ : connection_(connection), iov_index_(0), uint_buf_index_(0)
+ {}
+
+ SocketWriter& WriteUint32(uint32_t value) {
+ CHECK(uint_buf_index_ < kUintBufSize);
+ CHECK(iov_index_ < kIovSize);
+ uint32_t* ptr = uint_buf_ + uint_buf_index_;
+ uint_buf_[uint_buf_index_++] = value;
+ iov_[iov_index_].iov_base = ptr;
+ iov_[iov_index_].iov_len = sizeof(*ptr);
+ ++iov_index_;
+ return *this;
+ }
+
+ SocketWriter& WriteString(const char* value) {
+ uint32_t valuelen = strlen(value);
+ WriteUint32(valuelen);
+ if (valuelen == 0) {
+ return *this;
+ }
+
+ CHECK(iov_index_ < kIovSize);
+ iov_[iov_index_].iov_base = const_cast<char*>(value);
+ iov_[iov_index_].iov_len = valuelen;
+ ++iov_index_;
+
+ return *this;
+ }
+
+ bool Send() {
+ if (!connection_->IsValid()) {
+ return false;
+ }
+
+ if (writev(connection_->socket(), iov_, iov_index_) == -1) {
+ connection_->last_error_ = errno;
+ return false;
+ }
+
+ iov_index_ = uint_buf_index_ = 0;
+ return true;
+ }
+
+ private:
+ static constexpr size_t kUintBufSize = 8;
+ static constexpr size_t kIovSize = 8;
+
+ PropertyServiceConnection* connection_;
+ iovec iov_[kIovSize];
+ size_t iov_index_;
+ uint32_t uint_buf_[kUintBufSize];
+ size_t uint_buf_index_;
+
+ DISALLOW_IMPLICIT_CONSTRUCTORS(SocketWriter);
};
struct prop_msg {
@@ -581,9 +621,9 @@
}
int result = -1;
- int fd = connection.GetFd();
+ int s = connection.socket();
- const int num_bytes = TEMP_FAILURE_RETRY(send(fd, msg, sizeof(prop_msg), 0));
+ const int num_bytes = TEMP_FAILURE_RETRY(send(s, msg, sizeof(prop_msg), 0));
if (num_bytes == sizeof(prop_msg)) {
// We successfully wrote to the property server but now we
// wait for the property server to finish its work. It
@@ -593,7 +633,7 @@
// once the socket closes. Out of paranoia we cap our poll
// at 250 ms.
pollfd pollfds[1];
- pollfds[0].fd = fd;
+ pollfds[0].fd = s;
pollfds[0].events = 0;
const int poll_result = TEMP_FAILURE_RETRY(poll(pollfds, 1, 250 /* ms */));
if (poll_result == 1 && (pollfds[0].revents & POLLHUP) != 0) {
@@ -1230,7 +1270,6 @@
detect_protocol_version();
}
- int result = -1;
if (g_propservice_protocol_version == kProtocolVersion1) {
// Old protocol does not support long names
if (strlen(key) >= PROP_NAME_MAX) return -1;
@@ -1241,27 +1280,60 @@
strlcpy(msg.name, key, sizeof msg.name);
strlcpy(msg.value, value, sizeof msg.value);
- result = send_prop_msg(&msg);
+ return send_prop_msg(&msg);
} else {
// Use proper protocol
PropertyServiceConnection connection;
- if (connection.IsValid() && connection.SendUint32(PROP_MSG_SETPROP2) &&
- connection.SendString(key) && connection.SendString(value) &&
- connection.RecvInt32(&result)) {
- if (result != PROP_SUCCESS) {
- __libc_format_log(ANDROID_LOG_WARN, "libc",
- "Unable to set property \"%s\" to \"%s\": error code: 0x%x", key, value,
- result);
- }
- } else {
- result = connection.GetLastError();
- __libc_format_log(ANDROID_LOG_WARN, "libc",
- "Unable to set property \"%s\" to \"%s\": error code: 0x%x (%s)", key,
- value, result, strerror(result));
+ if (!connection.IsValid()) {
+ errno = connection.GetLastError();
+ __libc_format_log(ANDROID_LOG_WARN,
+ "libc",
+ "Unable to set property \"%s\" to \"%s\": connection failed; errno=%d (%s)",
+ key,
+ value,
+ errno,
+ strerror(errno));
+ return -1;
}
- }
- return result != 0 ? -1 : 0;
+ SocketWriter writer(&connection);
+ if (!writer.WriteUint32(PROP_MSG_SETPROP2).WriteString(key).WriteString(value).Send()) {
+ errno = connection.GetLastError();
+ __libc_format_log(ANDROID_LOG_WARN,
+ "libc",
+ "Unable to set property \"%s\" to \"%s\": write failed; errno=%d (%s)",
+ key,
+ value,
+ errno,
+ strerror(errno));
+ return -1;
+ }
+
+ int result = -1;
+ if (!connection.RecvInt32(&result)) {
+ errno = connection.GetLastError();
+ __libc_format_log(ANDROID_LOG_WARN,
+ "libc",
+ "Unable to set property \"%s\" to \"%s\": recv failed; errno=%d (%s)",
+ key,
+ value,
+ errno,
+ strerror(errno));
+ return -1;
+ }
+
+ if (result != PROP_SUCCESS) {
+ __libc_format_log(ANDROID_LOG_WARN,
+ "libc",
+ "Unable to set property \"%s\" to \"%s\": error code: 0x%x",
+ key,
+ value,
+ result);
+ return -1;
+ }
+
+ return 0;
+ }
}
int __system_property_update(prop_info* pi, const char* value, unsigned int len) {