summaryrefslogtreecommitdiff
path: root/osi
diff options
context:
space:
mode:
authorPavlin Radoslavov <pavlin@google.com>2016-05-12 11:36:44 -0700
committerPavlin Radoslavov <pavlin@google.com>2016-05-12 14:59:29 -0700
commit574dcfb73e3741d715f7d4394fe5d3bd587cb0d2 (patch)
tree8c3139db8b85b8733e47eb74537a8fbfffed7218 /osi
parent42e54009d789b5cae6df4599013852a52c1079a1 (diff)
downloadandroid-system-bt-574dcfb73e3741d715f7d4394fe5d3bd587cb0d2.tar.gz
android-system-bt-574dcfb73e3741d715f7d4394fe5d3bd587cb0d2.tar.xz
Restart failed system calls interrupted with errno of EINTR
In number of places we don't handle properly system calls failures when the errno is EINTR (i.e., the system call was interrupted by a signal). In all our use cases, the system calls should be restarted. The handling of the following system calls (as used in the code) has been updated/fixed: poll, send, recv, sendmsg, nanosleep, epoll_wait read - mostly (e.g., socket-like fds) write - mostly (e.g., socket-like fds) select, accept, connect Bug: 28471477 Bug: 28658141 Change-Id: I03e6f0f67e33876780fb6d02c33eb84547ba8f95
Diffstat (limited to 'osi')
-rw-r--r--osi/include/osi.h3
-rw-r--r--osi/include/socket.h24
-rw-r--r--osi/src/eager_reader.c26
-rw-r--r--osi/src/metrics.cpp9
-rw-r--r--osi/src/reactor.c5
-rw-r--r--osi/src/socket.c17
-rw-r--r--osi/src/socket_utils/socket_local_client.c3
7 files changed, 57 insertions, 30 deletions
diff --git a/osi/include/osi.h b/osi/include/osi.h
index 8cdf808..cd9887c 100644
--- a/osi/include/osi.h
+++ b/osi/include/osi.h
@@ -59,3 +59,6 @@
// No guarantees of distribution are made.
// Effort is made for this to come from a real random source.
int osi_rand(void);
+
+// Re-run |fn| system call until the system call doesn't cause EINTR.
+#define OSI_NO_INTR(fn) do {} while ((fn) == -1 && errno == EINTR)
diff --git a/osi/include/socket.h b/osi/include/socket.h
index 2833290..c5b2b25 100644
--- a/osi/include/socket.h
+++ b/osi/include/socket.h
@@ -58,20 +58,24 @@ socket_t *socket_accept(const socket_t *socket);
// block. This function returns a positive integer representing the number
// of bytes copied into |buf| on success, 0 if the socket has disconnected,
// and -1 on error. This function may return a value less than |count| if not
-// enough data is currently available. If this function returns -1, errno will also
-// be set (see recv(2) for possible errno values). If there were no bytes available
-// to be read, this function returns -1 and sets errno to EWOULDBLOCK. Neither
-// |socket| nor |buf| may be NULL.
+// enough data is currently available. If this function returns -1, errno will
+// also be set (see recv(2) for possible errno values). However, if the reading
+// system call was interrupted with errno of EINTR, the read operation is
+// restarted internally without propagating EINTR back to the caller. If there
+// were no bytes available to be read, this function returns -1 and sets errno
+// to EWOULDBLOCK. Neither |socket| nor |buf| may be NULL.
ssize_t socket_read(const socket_t *socket, void *buf, size_t count);
// Writes up to |count| bytes from |buf| into |socket|. This function will not
// block. Returns a positive integer representing the number of bytes written
-// to |socket| on success, 0 if the socket has disconnected, and -1 on error. This
-// function may return a value less than |count| if writing more bytes would result
-// in blocking. If this function returns -1, errno will also be set (see send(2) for
-// possible errno values). If no bytes could be written without blocking, this
-// function will return -1 and set errno to EWOULDBLOCK. Neither |socket| nor |buf|
-// may be NULL.
+// to |socket| on success, 0 if the socket has disconnected, and -1 on error.
+// This function may return a value less than |count| if writing more bytes
+// would result in blocking. If this function returns -1, errno will also be
+// set (see send(2) for possible errno values). However, if the writing system
+// call was interrupted with errno of EINTR, the write operation is restarted
+// internally without propagating EINTR back to the caller. If no bytes could
+// be written without blocking, this function will return -1 and set errno to
+// EWOULDBLOCK. Neither |socket| nor |buf| may be NULL.
ssize_t socket_write(const socket_t *socket, const void *buf, size_t count);
// This function performs the same write operation as |socket_write| and also
diff --git a/osi/src/eager_reader.c b/osi/src/eager_reader.c
index 044ee81..07a944c 100644
--- a/osi/src/eager_reader.c
+++ b/osi/src/eager_reader.c
@@ -215,15 +215,23 @@ static bool has_byte(const eager_reader_t *reader) {
assert(reader != NULL);
fd_set read_fds;
- FD_ZERO(&read_fds);
- FD_SET(reader->bytes_available_fd, &read_fds);
- // Immediate timeout
- struct timeval timeout;
- timeout.tv_sec = 0;
- timeout.tv_usec = 0;
+ for (;;) {
+ FD_ZERO(&read_fds);
+ FD_SET(reader->bytes_available_fd, &read_fds);
+
+ // Immediate timeout
+ struct timeval timeout;
+ timeout.tv_sec = 0;
+ timeout.tv_usec = 0;
+
+ int ret = select(reader->bytes_available_fd + 1, &read_fds, NULL, NULL,
+ &timeout);
+ if (ret == -1 && errno == EINTR)
+ continue;
+ break;
+ }
- select(reader->bytes_available_fd + 1, &read_fds, NULL, NULL, &timeout);
return FD_ISSET(reader->bytes_available_fd, &read_fds);
}
@@ -239,7 +247,9 @@ static void inbound_data_waiting(void *context) {
buffer->length = 0;
buffer->offset = 0;
- int bytes_read = read(reader->inbound_fd, buffer->data, reader->buffer_size);
+ ssize_t bytes_read;
+ OSI_NO_INTR(bytes_read = read(reader->inbound_fd, buffer->data,
+ reader->buffer_size));
if (bytes_read > 0) {
// Save the data for later
buffer->length = bytes_read;
diff --git a/osi/src/metrics.cpp b/osi/src/metrics.cpp
index a99200b..064f99e 100644
--- a/osi/src/metrics.cpp
+++ b/osi/src/metrics.cpp
@@ -196,7 +196,9 @@ void metrics_write(int fd, bool clear) {
std::string protoBase64;
base::Base64Encode(serialized, &protoBase64);
- if (write(fd, protoBase64.c_str(), protoBase64.size()) == -1) {
+ ssize_t ret;
+ OSI_NO_INTR(ret = write(fd, protoBase64.c_str(), protoBase64.size()));
+ if (ret == -1) {
LOG_ERROR(LOG_TAG, "%s: error writing to dumpsys fd: %s (%d)", __func__,
strerror(errno), errno);
}
@@ -215,9 +217,10 @@ void metrics_print(int fd, bool clear) {
}
log_lock.unlock();
- if (write(fd, pretty_output.c_str(), pretty_output.size()) == -1) {
+ ssize_t ret;
+ OSI_NO_INTR(ret = write(fd, pretty_output.c_str(), pretty_output.size()));
+ if (ret == -1) {
LOG_ERROR(LOG_TAG, "%s: error writing to dumpsys fd: %s (%d)", __func__,
strerror(errno), errno);
}
-
}
diff --git a/osi/src/reactor.c b/osi/src/reactor.c
index 7bab5e4..7388d52 100644
--- a/osi/src/reactor.c
+++ b/osi/src/reactor.c
@@ -237,10 +237,7 @@ static reactor_status_t run_reactor(reactor_t *reactor, int iterations) {
pthread_mutex_unlock(&reactor->list_lock);
int ret;
- do {
- ret = epoll_wait(reactor->epoll_fd, events, MAX_EVENTS, -1);
- } while (ret == -1 && errno == EINTR);
-
+ OSI_NO_INTR(ret = epoll_wait(reactor->epoll_fd, events, MAX_EVENTS, -1));
if (ret == -1) {
LOG_ERROR(LOG_TAG, "%s error in epoll_wait: %s", __func__, strerror(errno));
reactor->is_running = false;
diff --git a/osi/src/socket.c b/osi/src/socket.c
index 80813f5..4ebb3b8 100644
--- a/osi/src/socket.c
+++ b/osi/src/socket.c
@@ -113,7 +113,8 @@ bool socket_listen(const socket_t *socket, port_t port) {
socket_t *socket_accept(const socket_t *socket) {
assert(socket != NULL);
- int fd = accept(socket->fd, NULL, NULL);
+ int fd;
+ OSI_NO_INTR(fd = accept(socket->fd, NULL, NULL));
if (fd == INVALID_FD) {
LOG_ERROR(LOG_TAG, "%s unable to accept socket: %s", __func__, strerror(errno));
return NULL;
@@ -129,14 +130,20 @@ ssize_t socket_read(const socket_t *socket, void *buf, size_t count) {
assert(socket != NULL);
assert(buf != NULL);
- return recv(socket->fd, buf, count, MSG_DONTWAIT);
+ ssize_t ret;
+ OSI_NO_INTR(ret = recv(socket->fd, buf, count, MSG_DONTWAIT));
+
+ return ret;
}
ssize_t socket_write(const socket_t *socket, const void *buf, size_t count) {
assert(socket != NULL);
assert(buf != NULL);
- return send(socket->fd, buf, count, MSG_DONTWAIT);
+ ssize_t ret;
+ OSI_NO_INTR(ret = send(socket->fd, buf, count, MSG_DONTWAIT));
+
+ return ret;
}
ssize_t socket_write_and_transfer_fd(const socket_t *socket, const void *buf, size_t count, int fd) {
@@ -166,7 +173,9 @@ ssize_t socket_write_and_transfer_fd(const socket_t *socket, const void *buf, si
header->cmsg_len = CMSG_LEN(sizeof(int));
*(int *)CMSG_DATA(header) = fd;
- ssize_t ret = sendmsg(socket->fd, &msg, MSG_DONTWAIT);
+ ssize_t ret;
+ OSI_NO_INTR(ret = sendmsg(socket->fd, &msg, MSG_DONTWAIT));
+
close(fd);
return ret;
}
diff --git a/osi/src/socket_utils/socket_local_client.c b/osi/src/socket_utils/socket_local_client.c
index ccfe743..73a828c 100644
--- a/osi/src/socket_utils/socket_local_client.c
+++ b/osi/src/socket_utils/socket_local_client.c
@@ -121,7 +121,8 @@ int osi_socket_local_client_connect(int fd, const char *name, int namespaceId,
goto error;
}
- if (connect(fd, (struct sockaddr *)&addr, alen) < 0) {
+ OSI_NO_INTR(err = connect(fd, (struct sockaddr *)&addr, alen));
+ if (err < 0) {
goto error;
}