[trafficserver] branch master updated: Log meta file: ensure the entire contents are written (#8062)

[bn...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bneradt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 870bbdb  Log meta file: ensure the entire contents are written (#8062)
870bbdb is described below

commit 870bbdb18eb267e4fd0ebebf3d667ac0dd1d5479
Author: Brian Neradt <br...@gmail.com>
AuthorDate: Thu Jul 15 16:05:21 2021 -0500

    Log meta file: ensure the entire contents are written (#8062)
    
    Updating the write of the meta file content for log rotation to ensure
    that the entire line is written. If not, then the file will be
    ill-formed which will lead eventually to an assertion when we try to
    read from it later.
---
 include/tscore/ink_sock.h | 32 ++++++++++++++++++++++++++++++++
 src/tscore/BaseLogFile.cc | 21 ++++++++++++---------
 src/tscore/ink_sock.cc    | 43 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 9 deletions(-)

diff --git a/include/tscore/ink_sock.h b/include/tscore/ink_sock.h
index 3ea127e..6fff029 100644
--- a/include/tscore/ink_sock.h
+++ b/include/tscore/ink_sock.h
@@ -41,6 +41,18 @@ int safe_listen(int s, int backlog);
 int safe_getsockname(int s, struct sockaddr *name, int *namelen);
 int safe_getpeername(int s, struct sockaddr *name, int *namelen);
 
+/** Repeat write calls to fd until all len bytes are written.
+ *
+ * @param[in] fd The file descriptor to write to.
+ * @param[in] buffer The buffer of bytes to write into fd.
+ * @param[in] len The number of bytes in buffer to write into fd.
+ *
+ * @return The number of bytes written or -1 if there was an error writing to
+ * the file descriptor. If -1 is returned, errno should indicate why the
+ * write failed.
+ */
+int safe_write(int fd, const char *buffer, int len);
+
 int safe_fcntl(int fd, int cmd, int arg);
 int safe_ioctl(int fd, int request, char *arg);
 
@@ -50,8 +62,28 @@ int safe_clr_fl(int fd, int arg);
 int safe_blocking(int fd);
 int safe_nonblocking(int fd);
 
+/** Poll on fd until it is ready for reading.
+ *
+ * @param[in] fd The file descriptor to poll on.
+ * @param[in] timeout_msec How many milliseconds to poll on the socket before
+ *   giving up.
+ *
+ * @return 1 if the socket is ready for reading, -1 if there was an error, and
+ * 0 for timeout.
+ */
 int read_ready(int fd, int timeout_msec = 0);
 
+/** Poll on fd until it is ready for writing.
+ *
+ * @param[in] fd The file descriptor to poll on.
+ * @param[in] timeout_msec How many milliseconds to poll on the socket before
+ *   giving up.
+ *
+ * @return 1 if the socket is ready for writing, -1 if there was an error, and
+ * 0 for timeout.
+ */
+int write_ready(int fd, int timeout_msec = 0);
+
 char fd_read_char(int fd);
 int fd_read_line(int fd, char *s, int len);
 
diff --git a/src/tscore/BaseLogFile.cc b/src/tscore/BaseLogFile.cc
index 280ad17..1c42f20 100644
--- a/src/tscore/BaseLogFile.cc
+++ b/src/tscore/BaseLogFile.cc
@@ -22,6 +22,7 @@
  */
 
 #include "tscore/BaseLogFile.h"
+#include "tscore/ink_sock.h"
 
 /*
  * This constructor creates a BaseLogFile based on a given name.
@@ -532,24 +533,23 @@ BaseMetaInfo::_write_to_file()
   }
   log_log_trace("Successfully opened metafile=%s\n", _filename);
 
-  int n;
   if (_flags & VALID_CREATION_TIME) {
     log_log_trace("Writing creation time to %s\n", _filename);
-    n = snprintf(_buffer, BUF_SIZE, "creation_time = %lu\n", static_cast<unsigned long>(_creation_time));
+    int const num_to_write = snprintf(_buffer, BUF_SIZE, "creation_time = %lu\n", static_cast<unsigned long>(_creation_time));
     // TODO modify this runtime check so that it is not an assertion
-    ink_release_assert(n <= BUF_SIZE);
-    if (write(fd, _buffer, n) == -1) {
-      log_log_trace("Could not write creation_time");
+    ink_release_assert(num_to_write <= BUF_SIZE);
+    if (safe_write(fd, _buffer, num_to_write) == -1) {
+      log_log_error("Could not write creation_time: %s\n", strerror(errno));
     }
   }
 
   if (_flags & VALID_SIGNATURE) {
     log_log_trace("Writing signature to %s\n", _filename);
-    n = snprintf(_buffer, BUF_SIZE, "object_signature = %" PRIu64 "\n", _log_object_signature);
+    int const num_to_write = snprintf(_buffer, BUF_SIZE, "object_signature = %" PRIu64 "\n", _log_object_signature);
     // TODO modify this runtime check so that it is not an assertion
-    ink_release_assert(n <= BUF_SIZE);
-    if (write(fd, _buffer, n) == -1) {
-      log_log_error("Could not write object_signature\n");
+    ink_release_assert(num_to_write <= BUF_SIZE);
+    if (safe_write(fd, _buffer, num_to_write) == -1) {
+      log_log_error("Could not write object_signature: %s\n", strerror(errno));
     }
     log_log_trace("BaseMetaInfo::_write_to_file\n"
                   "\tfilename = %s\n"
@@ -558,6 +558,9 @@ BaseMetaInfo::_write_to_file()
                   _filename, _log_object_signature, _buffer);
   }
 
+  if (fsync(fd) == -1) {
+    log_log_error("Could not fsync the log meta file: %s\n", strerror(errno));
+  }
   close(fd);
 }
 
diff --git a/src/tscore/ink_sock.cc b/src/tscore/ink_sock.cc
index dbc0ae2..20332c4 100644
--- a/src/tscore/ink_sock.cc
+++ b/src/tscore/ink_sock.cc
@@ -27,6 +27,7 @@
 
 
 ***************************************************************************/
+#include "tscore/ink_sock.h"
 #include "tscore/ink_platform.h"
 #include "tscore/ink_assert.h"
 #include "tscore/ink_string.h"
@@ -112,6 +113,29 @@ safe_clr_fl(int fd, int arg)
 }
 
 int
+safe_write(int fd, const char *buffer, int len)
+{
+  int num_written = 0;
+  while (num_written < len) {
+    int const ret = write(fd, buffer + num_written, len - num_written);
+    if (ret == -1) {
+      if (errno == EAGAIN || errno == EINTR) {
+        // The errno indicates that we should attempt the write again. Poll
+        // until the socket is ready for writing then try again.
+        if (write_ready(fd) != 1) {
+          return -1;
+        }
+        continue;
+      }
+      // Some unexpected errno was encountered.
+      return ret;
+    }
+    num_written += ret;
+  }
+  return num_written;
+}
+
+int
 safe_nonblocking(int fd)
 {
   return safe_set_fl(fd, O_NONBLOCK);
@@ -143,6 +167,25 @@ read_ready(int fd, int timeout_msec)
 }
 
 int
+write_ready(int fd, int timeout_msec)
+{
+  struct pollfd p;
+  p.events = POLLOUT;
+  p.fd     = fd;
+  int r    = poll(&p, 1, timeout_msec);
+  if (r <= 0) {
+    return r;
+  }
+  if (p.revents & (POLLERR | POLLNVAL)) {
+    return -1;
+  }
+  if (p.revents & POLLOUT) {
+    return 1;
+  }
+  return 0;
+}
+
+int
 safe_ioctl(int fd, int request, char *arg)
 {
   int r = -1;