Fix race condition in FileLogger (#378)

Signed-off-by: Addisu Z. Taddese <[email protected]>
gazebosim · Jun 24, 2022 · eda17f5 · eda17f5
1 parent 7be12d1
commit eda17f5
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 5 deletions.
diff --git a/include/ignition/common/Console.hh b/include/ignition/common/Console.hh
@@ -113,13 +113,25 @@ namespace ignition
                    /// \brief Destructor.
                    public: virtual ~Buffer();
 
+                   /// \brief Writes _count characters to the string buffer
+                   /// \param[in] _char Input rharacter array.
+                   /// \param[in] _count Number of characters in array.
+                   /// \return The number of characters successfully written.
+                   public: std::streamsize xsputn(
+                        const char *_char, std::streamsize _count) override;
+
                    /// \brief Sync the stream (output the string buffer
                    /// contents).
                    /// \return Return 0 on success.
-                   public: virtual int sync();
+                   public: int sync() override;
 
                    /// \brief Stream to output information into.
                    public: std::ofstream *stream;
+                   /// \brief Mutex to synchronize writes to the string buffer
+                   /// and the output stream.
+                   /// \todo(nkoenig) Put this back in for ign-common5, and
+                   /// remove the corresponding static version.
+                   // public: std::mutex syncMutex;
                  };
 
       IGN_COMMON_WARN_IGNORE__DLL_INTERFACE_MISSING

diff --git a/src/Console.cc b/src/Console.cc
@@ -362,16 +362,32 @@ FileLogger::Buffer::~Buffer()
     static_cast<std::ofstream*>(this->stream)->close();
 }
 
+/////////////////////////////////////////////////
+std::streamsize FileLogger::Buffer::xsputn(const char *_char,
+                                           std::streamsize _count)
+{
+  auto lk = BufferLock(reinterpret_cast<std::uintptr_t>(this));
+  return std::stringbuf::xsputn(_char, _count);
+}
+
 /////////////////////////////////////////////////
 int FileLogger::Buffer::sync()
 {
   if (!this->stream)
     return -1;
 
-  *this->stream << this->str();
-
-  this->stream->flush();
+  {
+    auto lk = BufferLock(reinterpret_cast<std::uintptr_t>(this));
+    *this->stream << this->str();
+  }
 
-  this->str("");
+  {
+    auto lk = BufferLock(reinterpret_cast<std::uintptr_t>(this));
+    this->stream->flush();
+  }
+  {
+    auto lk = BufferLock(reinterpret_cast<std::uintptr_t>(this));
+    this->str("");
+  }
   return !(*this->stream);
 }