From: Mukund Sivaraman Date: Thu, 24 May 2012 05:33:45 +0000 (+0530) Subject: [1704] Remove separate class hierarchy for InterprocessSyncLocker X-Git-Tag: trac2351_base~226^2~60^2^2~61 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=2bf7dad904d16760987d3a0b27c02bb8a2b50e55;p=thirdparty%2Fkea.git [1704] Remove separate class hierarchy for InterprocessSyncLocker --- diff --git a/src/lib/log/logger_impl.cc b/src/lib/log/logger_impl.cc index c73503315c..7f5211e74b 100644 --- a/src/lib/log/logger_impl.cc +++ b/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 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"); } } diff --git a/src/lib/util/interprocess_sync.h b/src/lib/util/interprocess_sync.h index cc08d6a709..4ef36d5e96 100644 --- a/src/lib/util/interprocess_sync.h +++ b/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 diff --git a/src/lib/util/interprocess_sync_file.cc b/src/lib/util/interprocess_sync_file.cc index 190b84bea3..8ea2f94ee6 100644 --- a/src/lib/util/interprocess_sync_file.cc +++ b/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(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(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(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); diff --git a/src/lib/util/interprocess_sync_file.h b/src/lib/util/interprocess_sync_file.h index a71e4f9e03..4b9085c148 100644 --- a/src/lib/util/interprocess_sync_file.h +++ b/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 diff --git a/src/lib/util/tests/interprocess_sync_file_unittest.cc b/src/lib/util/tests/interprocess_sync_file_unittest.cc index 6d9fc3251b..3dd0ad0801 100644 --- a/src/lib/util/tests/interprocess_sync_file_unittest.cc +++ b/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