[1704] Remove separate class hierarchy for InterprocessSyncLocker

src/lib/log/logger_impl.cc

@@ -111,9 +111,9 @@ LoggerImpl::outputRaw(const Severity& severity, const string& message) {
     // Use a lock file for mutual exclusion from other processes to
     // avoid log messages getting interspersed
 
-    auto_ptr<InterprocessSyncLocker> locker(sync_->getLocker());
+    InterprocessSyncLocker locker(*sync_);
 
-    if (!locker->lock()) {
+    if (!locker.lock()) {
         LOG4CPLUS_ERROR(logger_, "Unable to lock logger lockfile");
     }
 
@@ -138,7 +138,7 @@ LoggerImpl::outputRaw(const Severity& severity, const string& message) {
             LOG4CPLUS_FATAL(logger_, message);
     }
 
-    if (!locker->unlock()) {
+    if (!locker.unlock()) {
         LOG4CPLUS_ERROR(logger_, "Unable to unlock logger lockfile");
     }
 }

src/lib/util/interprocess_sync.h

@@ -23,39 +23,52 @@ namespace util {
 class InterprocessSyncLocker;
 
 class InterprocessSync {
+friend class InterprocessSyncLocker;
 public:
     /// \brief Constructor
     ///
     /// Creates a interprocess synchronization object
     InterprocessSync(const std::string component_name) :
-        component_name_(component_name)
+        component_name_(component_name), is_locked_(false)
     {}
 
     /// \brief Destructor
     virtual ~InterprocessSync() {}
 
-    virtual InterprocessSyncLocker* getLocker() = 0;
-
 protected:
+    virtual bool lock() = 0;
+    virtual bool tryLock() = 0;
+    virtual bool unlock() = 0;
+
     std::string component_name_;
+    bool is_locked_;
 };
 
 class InterprocessSyncLocker {
-friend class InterprocessSync;
 public:
-    virtual bool lock() = 0;
-    virtual bool tryLock() = 0;
-    virtual bool unlock() = 0;
+    InterprocessSyncLocker(InterprocessSync& sync) :
+        sync_(sync)
+    {}
 
     /// \brief Destructor
-    virtual ~InterprocessSyncLocker() {}
+    ~InterprocessSyncLocker() {
+        unlock();
+    }
+
+    bool lock() {
+        return sync_.lock();
+    }
+
+    bool tryLock() {
+        return sync_.tryLock();
+    }
+
+    bool unlock() {
+        return sync_.unlock();
+    }
 
 protected:
-    InterprocessSyncLocker(InterprocessSync* sync) :
-        sync_(sync), is_locked_(false)
-    {}
-    InterprocessSync* sync_;
-    bool is_locked_;
+    InterprocessSync& sync_;
 };
 
 } // namespace util

src/lib/util/interprocess_sync_file.cc

@@ -58,49 +58,30 @@ InterprocessSyncFile::InterprocessSyncFile(const std::string component_name) :
 
 InterprocessSyncFile::~InterprocessSyncFile() {
     if (fd_ != -1) {
+        // This will also release any applied locks.
         close(fd_);
     }
 
     // The lockfile will continue to exist, and we must not delete it.
 }
 
-InterprocessSyncLocker*
-InterprocessSyncFile::getLocker() {
-    InterprocessSyncLocker* locker = new InterprocessSyncFileLocker(this);
-    return (locker);
-}
-
-///////////////////////////////////////////////////////////////////////////////////
-
-InterprocessSyncFileLocker::InterprocessSyncFileLocker(InterprocessSync* sync)
-    : InterprocessSyncLocker(sync)
-{
-}
-
-InterprocessSyncFileLocker::~InterprocessSyncFileLocker() {
-    unlock();
-}
-
 bool
-InterprocessSyncFileLocker::lock() {
+InterprocessSyncFile::lock() {
     if (is_locked_) {
         return (true);
     }
 
-    InterprocessSyncFile* sync = dynamic_cast<InterprocessSyncFile*>(sync_);
-    const int fd = sync->getFd();
-
-    if (fd != -1) {
+    if (fd_ != -1) {
         struct flock lock;
 
         // Acquire the exclusive lock
-        memset(&lock, 0, sizeof lock);
+        memset(&lock, 0, sizeof (lock));
         lock.l_type = F_WRLCK;
         lock.l_whence = SEEK_SET;
         lock.l_start = 0;
         lock.l_len = 1;
 
-        const int status = fcntl(fd, F_SETLKW, &lock);
+        const int status = fcntl(fd_, F_SETLKW, &lock);
         if (status == 0) {
             is_locked_ = true;
             return (true);
@@ -111,25 +92,22 @@ InterprocessSyncFileLocker::lock() {
 }
 
 bool
-InterprocessSyncFileLocker::tryLock() {
+InterprocessSyncFile::tryLock() {
     if (is_locked_) {
         return (true);
     }
 
-    InterprocessSyncFile* sync = dynamic_cast<InterprocessSyncFile*>(sync_);
-    const int fd = sync->getFd();
-
-    if (fd != -1) {
+    if (fd_ != -1) {
         struct flock lock;
 
         // Acquire the exclusive lock
-        memset(&lock, 0, sizeof lock);
+        memset(&lock, 0, sizeof (lock));
         lock.l_type = F_WRLCK;
         lock.l_whence = SEEK_SET;
         lock.l_start = 0;
         lock.l_len = 1;
 
-        const int status = fcntl(fd, F_SETLK, &lock);
+        const int status = fcntl(fd_, F_SETLK, &lock);
         if (status == 0) {
             is_locked_ = true;
             return (true);
@@ -140,25 +118,22 @@ InterprocessSyncFileLocker::tryLock() {
 }
 
 bool
-InterprocessSyncFileLocker::unlock() {
+InterprocessSyncFile::unlock() {
     if (!is_locked_) {
         return (true);
     }
 
-    InterprocessSyncFile *sync = dynamic_cast<InterprocessSyncFile*>(sync_);
-    const int fd = sync->getFd();
-
-    if (fd != -1) {
+    if (fd_ != -1) {
         struct flock lock;
 
         // Release the exclusive lock
-        memset(&lock, 0, sizeof lock);
+        memset(&lock, 0, sizeof (lock));
         lock.l_type = F_UNLCK;
         lock.l_whence = SEEK_SET;
         lock.l_start = 0;
         lock.l_len = 1;
 
-        const int status = fcntl(fd, F_SETLKW, &lock);
+        const int status = fcntl(fd_, F_SETLKW, &lock);
         if (status == 0) {
             is_locked_ = false;
             return (true);

src/lib/util/interprocess_sync_file.h

@@ -40,28 +40,13 @@ public:
     /// \brief Destructor
     ~InterprocessSyncFile();
 
-    InterprocessSyncLocker* getLocker();
-
-    int getFd() const {
-        return (fd_);
-    }
-
-private:
-    int fd_;
-};
-
-class InterprocessSyncFileLocker : public InterprocessSyncLocker {
-friend class InterprocessSyncFile;
-public:
-    /// \brief Destructor
-    ~InterprocessSyncFileLocker();
-
+protected:
     bool lock();
     bool tryLock();
     bool unlock();
 
-protected:
-    InterprocessSyncFileLocker(InterprocessSync* sync);
+private:
+    int fd_;
 };
 
 } // namespace util

src/lib/util/tests/interprocess_sync_file_unittest.cc

@@ -26,10 +26,10 @@ protected:
 };
 
 TEST_F(InterprocessSyncFileTest, TestLock) {
-  InterprocessSync* sync = new InterprocessSyncFile("test");
-  InterprocessSyncLocker* locker = sync->getLocker();
+  InterprocessSyncFile sync("test");
+  InterprocessSyncLocker locker(sync);
 
-  EXPECT_TRUE(locker->lock());
+  EXPECT_TRUE(locker.lock());
 
   int fds[2];
 
@@ -49,16 +49,13 @@ TEST_F(InterprocessSyncFileTest, TestLock) {
       // Child writes to pipe
       close(fds[0]);
 
-      InterprocessSync* sync2 = new InterprocessSyncFile("test");
-      InterprocessSyncLocker* locker2 = sync2->getLocker();
+      InterprocessSyncFile sync2("test");
+      InterprocessSyncLocker locker2(sync2);
 
-      if (!locker2->tryLock()) {
+      if (!locker2.tryLock()) {
           locked = 1;
       }
 
-      delete locker2;
-      delete sync2;
-
       write(fds[1], &locked, sizeof(locked));
       close(fds[1]);
       exit(0);
@@ -79,36 +76,28 @@ TEST_F(InterprocessSyncFileTest, TestLock) {
   }
 
   EXPECT_TRUE(was_locked);
-  EXPECT_TRUE(locker->unlock());
-
-  delete locker;
-  delete sync;
+  EXPECT_TRUE(locker.unlock());
 }
 
 TEST_F(InterprocessSyncFileTest, TestMultipleFilesDirect) {
-  InterprocessSync* sync = new InterprocessSyncFile("test1");
-  InterprocessSyncLocker* locker = sync->getLocker();
-
-  EXPECT_TRUE(locker->lock());
+  InterprocessSyncFile sync("test1");
+  InterprocessSyncLocker locker(sync);
 
-  InterprocessSync* sync2 = new InterprocessSyncFile("test2");
-  InterprocessSyncLocker* locker2 = sync2->getLocker();
-  EXPECT_TRUE(locker2->lock());
-  EXPECT_TRUE(locker2->unlock());
-  delete sync2;
-  delete locker2;
+  EXPECT_TRUE(locker.lock());
 
-  EXPECT_TRUE(locker->unlock());
+  InterprocessSyncFile sync2("test2");
+  InterprocessSyncLocker locker2(sync2);
+  EXPECT_TRUE(locker2.lock());
+  EXPECT_TRUE(locker2.unlock());
 
-  delete locker;
-  delete sync;
+  EXPECT_TRUE(locker.unlock());
 }
 
 TEST_F(InterprocessSyncFileTest, TestMultipleFilesForked) {
-  InterprocessSync* sync = new InterprocessSyncFile("test");
-  InterprocessSyncLocker* locker = sync->getLocker();
+  InterprocessSyncFile sync("test");
+  InterprocessSyncLocker locker(sync);
 
-  EXPECT_TRUE(locker->lock());
+  EXPECT_TRUE(locker.lock());
 
   int fds[2];
 
@@ -121,16 +110,13 @@ TEST_F(InterprocessSyncFileTest, TestMultipleFilesForked) {
       // Child writes to pipe
       close(fds[0]);
 
-      InterprocessSync* sync2 = new InterprocessSyncFile("test2");
-      InterprocessSyncLocker* locker2 = sync2->getLocker();
+      InterprocessSyncFile sync2("test2");
+      InterprocessSyncLocker locker2(sync2);
 
-      if (locker2->tryLock()) {
+      if (locker2.tryLock()) {
           locked = 0;
       }
 
-      delete locker2;
-      delete sync2;
-
       write(fds[1], &locked, sizeof(locked));
       close(fds[1]);
       exit(0);
@@ -151,10 +137,7 @@ TEST_F(InterprocessSyncFileTest, TestMultipleFilesForked) {
   }
 
   EXPECT_TRUE(was_not_locked);
-  EXPECT_TRUE(locker->unlock());
-
-  delete locker;
-  delete sync;
+  EXPECT_TRUE(locker.unlock());
 }
 
 } // namespace util