[libc-commits] [libc] [libc] Lock the output stream for the 'puts' call (PR #76513)

[Joseph Huber via libc-commits]

https://github.com/jhuber6 updated https://github.com/llvm/llvm-project/pull/76513

>From 1a57da134a9822a7e7a7d3e97599fa8b8845f698 Mon Sep 17 00:00:00 2001
From: Joseph Huber <huberjn at outlook.com>
Date: Thu, 28 Dec 2023 10:32:33 -0600
Subject: [PATCH 1/2] [libc] Lock the output stream for the 'puts' call

Summary:
The `puts` function consists of an initial write and then another write
to append the newline. When executing code in parallel, it is possible
for these writes to becomes disjointed. This code adds an explicit lock
call to ensure that the string is always appended by the newline as the
users expects.

Wasn't sure if this required a test as it would be difficult since
reproducing it would be flaky.
---
 libc/src/stdio/generic/puts.cpp | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/libc/src/stdio/generic/puts.cpp b/libc/src/stdio/generic/puts.cpp
index d8d69332566cd0..8e402c0ab072da 100644
--- a/libc/src/stdio/generic/puts.cpp
+++ b/libc/src/stdio/generic/puts.cpp
@@ -15,8 +15,25 @@
 
 namespace LIBC_NAMESPACE {
 
+namespace {
+
+// Simple helper to unlock the file once destroyed.
+struct ScopedLock {
+  ScopedLock(LIBC_NAMESPACE::File *stream) : stream(stream) { stream->lock(); }
+  ~ScopedLock() { stream->unlock(); }
+
+private:
+  LIBC_NAMESPACE::File *stream;
+};
+
+} // namespace
+
 LLVM_LIBC_FUNCTION(int, puts, (const char *__restrict str)) {
   cpp::string_view str_view(str);
+
+  // We need to lock the stream to ensure the newline is always appended.
+  ScopedLock lock(LIBC_NAMESPACE::stdout);
+
   auto result = LIBC_NAMESPACE::stdout->write(str, str_view.size());
   if (result.has_error())
     libc_errno = result.error;

>From 164048b277febc928b6ae9a25d79456deb2d8903 Mon Sep 17 00:00:00 2001
From: Joseph Huber <huberjn at outlook.com>
Date: Tue, 2 Jan 2024 10:01:37 -0600
Subject: [PATCH 2/2] Use unlocked

---
 libc/src/stdio/generic/puts.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libc/src/stdio/generic/puts.cpp b/libc/src/stdio/generic/puts.cpp
index 8e402c0ab072da..0f19ec52676712 100644
--- a/libc/src/stdio/generic/puts.cpp
+++ b/libc/src/stdio/generic/puts.cpp
@@ -34,7 +34,7 @@ LLVM_LIBC_FUNCTION(int, puts, (const char *__restrict str)) {
   // We need to lock the stream to ensure the newline is always appended.
   ScopedLock lock(LIBC_NAMESPACE::stdout);
 
-  auto result = LIBC_NAMESPACE::stdout->write(str, str_view.size());
+  auto result = LIBC_NAMESPACE::stdout->write_unlocked(str, str_view.size());
   if (result.has_error())
     libc_errno = result.error;
   size_t written = result.value;
@@ -42,7 +42,7 @@ LLVM_LIBC_FUNCTION(int, puts, (const char *__restrict str)) {
     // The stream should be in an error state in this case.
     return EOF;
   }
-  result = LIBC_NAMESPACE::stdout->write("\n", 1);
+  result = LIBC_NAMESPACE::stdout->write_unlocked("\n", 1);
   if (result.has_error())
     libc_errno = result.error;
   written = result.value;