ProcStats : Use mutex lock to make fetch() thread safe.

          : Also minor cleanup and general code modernisation.

GaudiAud/src/ProcStats.cpp

@@ -18,6 +18,7 @@
 #endif
 
 #include "ProcStats.h"
+#include <memory>
 
 #if defined( __linux__ ) or defined( __APPLE__ )
 #  include <iostream>
@@ -239,55 +240,50 @@ struct linux_proc {
 };
 #endif // __linux__ or __APPLE__
 
-ProcStats::cleanup::~cleanup() {
-  if ( ProcStats::inst != 0 ) {
-    delete ProcStats::inst;
-    ProcStats::inst = 0;
-  }
-}
-
 ProcStats* ProcStats::instance() {
-  static cleanup c;
-  if ( !inst ) inst = new ProcStats;
-  return inst;
+  static std::once_flag             alloc_instance_once;
+  static std::unique_ptr<ProcStats> inst;
+  std::call_once( alloc_instance_once, []() { inst = std::make_unique<ProcStats>(); } );
+  return inst.get();
 }
 
-ProcStats* ProcStats::inst = 0;
-
-ProcStats::ProcStats() : valid( false ) {
+ProcStats::ProcStats() {
 #if defined( __linux__ ) or defined( __APPLE__ )
-  pg_size = sysconf( _SC_PAGESIZE ); // getpagesize();
+  m_pg_size = sysconf( _SC_PAGESIZE ); // getpagesize();
 
-  fname = "/proc/" + std::to_string( getpid() ) + "/stat";
+  m_fname = "/proc/" + std::to_string( getpid() ) + "/stat";
 
-  fd.open( fname.c_str(), O_RDONLY );
-  if ( !fd ) {
-    cerr << "Failed to open " << fname << endl;
+  m_ufd.open( m_fname.c_str(), O_RDONLY );
+  if ( !m_ufd ) {
+    cerr << "Failed to open " << m_fname << endl;
     return;
   }
 #endif // __linux__ or __APPLE__
-  valid = true;
+  m_valid = true;
 }
 
 bool ProcStats::fetch( procInfo& f ) {
-  if ( valid == false ) return false;
+  if ( !m_valid ) { return false; }
 
 #if defined( __linux__ ) or defined( __APPLE__ )
-  double     pr_size, pr_rssize;
+
+  std::scoped_lock lock{ m_mutex };
+
+  double     pr_size{ 0 }, pr_rssize{ 0 };
   linux_proc pinfo;
-  int        cnt;
+  int        cnt{ 0 };
 
-  fd.lseek( 0, SEEK_SET );
+  m_ufd.lseek( 0, SEEK_SET );
 
-  if ( ( cnt = fd.read( buf, sizeof( buf ) ) ) < 0 ) {
+  if ( ( cnt = m_ufd.read( m_buf, sizeof( m_buf ) ) ) < 0 ) {
     cout << "LINUX Read of Proc file failed:" << endl;
     return false;
   }
 
   if ( cnt > 0 ) {
-    buf[std::min( static_cast<std::size_t>( cnt ), sizeof( buf ) - 1 )] = '\0';
+    m_buf[std::min( static_cast<std::size_t>( cnt ), sizeof( m_buf ) - 1 )] = '\0';
 
-    sscanf( buf,
+    sscanf( m_buf,
             // 1  2  3  4  5  6  7  8  9  10  1   2   3   4   5   6   7   8   9   20  1   2   3   4   5   6   7   8   9
             // 30  1   2   3   4   5
             "%d %s %c %d %d %d %d %d %lu %lu %lu %lu %lu %lu %lu %ld %ld %ld %ld %ld %ld %llu %lu %ld %lu %lu %lu %lu "
@@ -304,7 +300,7 @@ bool ProcStats::fetch( procInfo& f ) {
     pr_rssize = (double)pinfo.rss;
 
     f.vsize = pr_size / ( 1024 * 1024 );
-    f.rss   = pr_rssize * pg_size / ( 1024 * 1024 );
+    f.rss   = pr_rssize * m_pg_size / ( 1024 * 1024 );
   }
 
 #else
@@ -312,10 +308,10 @@ bool ProcStats::fetch( procInfo& f ) {
   f.rss   = 0;
 #endif // __linux__ or __APPLE__
 
-  bool rc = ( curr == f ) ? false : true;
+  const bool rc = !( m_curr == f );
 
-  curr.rss   = f.rss;
-  curr.vsize = f.vsize;
+  m_curr.rss   = f.rss;
+  m_curr.vsize = f.vsize;
 
   return rc;
 }

GaudiAud/src/ProcStats.h

@@ -8,15 +8,17 @@
 * granted to it by virtue of its status as an Intergovernmental Organization        *
 * or submit itself to any jurisdiction.                                             *
 \***********************************************************************************/
-#ifndef GAUDIAUD_PROCSTATS_H
-#define GAUDIAUD_PROCSTATS_H
+
+#pragma once
 
 // Class: ProcStats
 // Description:  Keeps statistics on memory usage
 // Author: Jim Kowalkowski (FNAL), modified by M. Shapiro (LBNL)
 
+#include <mutex>
 #include <string>
 #include <vector>
+
 #if defined( __linux__ ) or defined( __APPLE__ )
 #  include <fcntl.h>
 #  include <sys/stat.h>
@@ -25,7 +27,7 @@
 #endif // __linux__ or __APPLE__
 
 struct procInfo {
-  procInfo() : vsize( 0 ), rss( 0 ) {}
+  procInfo() = default;
   procInfo( double sz, double rss_sz ) : vsize( sz ), rss( rss_sz ) {}
 
   bool operator==( const procInfo& p ) const {
@@ -35,7 +37,7 @@ struct procInfo {
 #  pragma warning( disable : 1572 )
 #endif
 
-    return vsize == p.vsize && rss == p.rss;
+    return ( vsize == p.vsize && rss == p.rss );
 
 #ifdef __ICC
 // re-enable icc remark #1572
@@ -44,29 +46,23 @@ struct procInfo {
   }
 
   // see proc(4) man pages for units and a description
-  double vsize; // in MB (used to be in pages?)
-  double rss;   // in MB (used to be in pages?)
+  double vsize{ 0 }; // in MB (used to be in pages?)
+  double rss{ 0 };   // in MB (used to be in pages?)
 };
 
 class ProcStats {
 public:
   static ProcStats* instance();
 
-  bool   fetch( procInfo& fill_me );
-  double pageSize() const { return pg_size; }
-
-private:
+  bool fetch( procInfo& fill_me );
+  auto pageSize() const noexcept { return m_pg_size; }
   ProcStats();
 
-  struct cleanup {
-    cleanup() {}
-    ~cleanup();
-  };
-
-  friend struct cleanup;
-
+private:
   class unique_fd {
-    int m_fd;
+
+  private:
+    int m_fd{ -1 };
     unique_fd( const unique_fd& ) = delete;
     unique_fd& operator=( const unique_fd& ) = delete;
 
@@ -77,7 +73,7 @@ private:
       other.m_fd = -1;
     }
     ~unique_fd() {
-      if ( m_fd != -1 ) ::close( m_fd );
+      if ( m_fd != -1 ) { ::close( m_fd ); }
     }
 
     explicit operator bool() const { return m_fd != -1; }
@@ -86,29 +82,36 @@ private:
       m_fd = ::open( std::forward<Args>( args )... );
       return *this;
     }
+
+    int close() {
+      auto r = ::close( m_fd );
+      m_fd   = -1;
+      return r;
+    }
+
+  public:
 #define unique_fd_forward( fun )                                                                                       \
   template <typename... Args>                                                                                          \
-  auto fun( Args&&... args ) const->decltype( ::fun( m_fd, std::forward<Args>( args )... ) ) {                         \
+  auto fun( Args&&... args ) const {                                                                                   \
     return ::fun( m_fd, std::forward<Args>( args )... );                                                               \
   }
-    unique_fd_forward( lseek ) unique_fd_forward( read ) unique_fd_forward( write ) unique_fd_forward( fcntl )
-        unique_fd_forward( fsync ) unique_fd_forward( fchown ) unique_fd_forward( stat )
+    // clang-format off
+    unique_fd_forward( lseek )      
+    unique_fd_forward( read )   
+    unique_fd_forward( write )  
+    unique_fd_forward( fcntl )  
+    unique_fd_forward( fsync )  
+    unique_fd_forward( fchown ) 
+    unique_fd_forward( stat )
+    // clang-format on
 #undef unique_fd_forward
-            int close() {
-      auto r = ::close( m_fd );
-      m_fd   = -1;
-      return r;
-    }
   };
 
-  unique_fd   fd;
-  double      pg_size;
-  procInfo    curr;
-  std::string fname;
-  char        buf[500];
-  bool        valid;
-
-  static ProcStats* inst;
+  unique_fd   m_ufd;
+  double      m_pg_size{ 0 };
+  procInfo    m_curr;
+  std::string m_fname;
+  char        m_buf[500];
+  bool        m_valid{ false };
+  std::mutex  m_mutex;
 };
-
-#endif