From 33ce80f476be797a7c0839f463de2df05c97fe6d Mon Sep 17 00:00:00 2001 From: psychon Date: Mon, 5 Jul 2010 16:13:23 +0000 Subject: [PATCH] Use fcntl() instead of flock() for locking the config file fcntl() is more portable than flock() so this makes znc run on more systems (everyone smile and say hi to solaris). The downside is that fcntl() locks are lost if *any* fd referring to that file is closed (luckily we don't do that). The big downside is that the child process after fork() does not inherit the lock. To work around this, when znc forks into the background, the child process immediately blocks and tries to get the lock on the config file. Once the parent releases the lock by exiting, the child will get it. This shouldn't cause races with other ZNCs, because in every other place we don't block waiting for a lock but instead abort immediately if the file is already locked. Thanks to LeftWing aka Joshua M. Clulow for making znc work on solaris (and automatically fixing some issues with NFS in the process). git-svn-id: https://znc.svn.sourceforge.net/svnroot/znc/trunk@2065 726aef4b-f618-498e-8847-2d620e286838 --- FileUtils.cpp | 23 +++++++++++++++++------ FileUtils.h | 5 +++-- main.cpp | 10 ++++++++++ znc.cpp | 8 +++++++- znc.h | 1 + 5 files changed, 38 insertions(+), 9 deletions(-) diff --git a/FileUtils.cpp b/FileUtils.cpp index 961ec2af..a8112773 100644 --- a/FileUtils.cpp +++ b/FileUtils.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #ifndef HAVE_LSTAT # define lstat(a, b) stat(a, b) @@ -388,23 +389,33 @@ bool CFile::TryExLock(const CString& sLockFile, int iFlags) { } bool CFile::TryExLock() { - return Lock(LOCK_EX|LOCK_NB); + return Lock(F_WRLCK, false); +} + +bool CFile::ExLock() { + return Lock(F_WRLCK, true); } bool CFile::UnLock() { - return Lock(LOCK_UN); + return Lock(F_UNLCK, true); } -bool CFile::Lock(int iOperation) { +bool CFile::Lock(int iType, bool bBlocking) { + struct flock fl; + if (m_iFD == -1) { return false; } - if (flock(m_iFD, iOperation) != 0) { + fl.l_type = iType; + fl.l_whence = SEEK_SET; + fl.l_start = 0; + fl.l_len = 0; + if (fcntl(m_iFD, (bBlocking ? F_SETLKW : F_SETLK), &fl) == -1) { return false; + } else { + return true; } - - return true; } bool CFile::IsOpen() const { return (m_iFD != -1); } diff --git a/FileUtils.h b/FileUtils.h index e5e60cb6..482bc09f 100644 --- a/FileUtils.h +++ b/FileUtils.h @@ -111,6 +111,7 @@ public: bool TryExLock(const CString& sLockFile, int iFlags = O_RDONLY | O_CREAT); bool TryExLock(); + bool ExLock(); bool UnLock(); bool IsOpen() const; @@ -119,8 +120,8 @@ public: CString GetDir() const; private: - // flock() wrapper - bool Lock(int iOperation); + // fcntl() locking wrapper + bool Lock(int iType, bool bBlocking); CString m_sBuffer; int m_iFD; diff --git a/main.cpp b/main.cpp index 666cd8c6..20507a4c 100644 --- a/main.cpp +++ b/main.cpp @@ -254,6 +254,16 @@ int main(int argc, char** argv) { return 0; } + /* fcntl() locks don't necessarily propagate to forked() + * children. Reacquire the lock here. Use the blocking + * call to avoid race condition with parent exiting. + */ + if (!pZNC->WaitForChildLock()) { + CUtils::PrintError("Child was unable to obtain lock on config file."); + delete pZNC; + return 1; + } + // Redirect std in/out/err to /dev/null close(0); open("/dev/null", O_RDONLY); close(1); open("/dev/null", O_WRONLY); diff --git a/znc.cpp b/znc.cpp index 6d76d5b0..d0b5f950 100644 --- a/znc.cpp +++ b/znc.cpp @@ -1058,7 +1058,9 @@ bool CZNC::DoRehash(CString& sError) if (m_LockFile.IsOpen()) m_LockFile.Close(); - if (!m_LockFile.Open(m_sConfigFile)) { + // need to open the config file Read/Write for fcntl() + // exclusive locking to work properly! + if (!m_LockFile.Open(m_sConfigFile, O_RDWR)) { sError = "Can not open config file"; CUtils::PrintStatus(false, sError); return false; @@ -2071,3 +2073,7 @@ void CZNC::LeakConnectUser(CConnectUserTimer *pTimer) { if (m_pConnectUserTimer == pTimer) m_pConnectUserTimer = NULL; } + +bool CZNC::WaitForChildLock() { + return m_LockFile.ExLock(); +} diff --git a/znc.h b/znc.h index 38545975..fda9a764 100644 --- a/znc.h +++ b/znc.h @@ -38,6 +38,7 @@ public: void ReleaseISpoof(); bool WritePidFile(int iPid); bool DeletePidFile(); + bool WaitForChildLock(); Csock* FindSockByName(const CString& sSockName); bool IsHostAllowed(const CString& sHostMask) const; // This returns false if there are too many anonymous connections from this ip