commit 63b8ad8b7c6cc9845f11e5fbacf1a8cc9c977859
Author: David Faure <faure@kde.org>
Date: Sat Aug 6 14:53:36 2011 +0200
Implement locking on non-NFS systems using O_EXCL
Fixes locking on VFAT and CIFS etc.
The unit-test only tests local files automatically, but explains
how to set up test environments to test it on a FAT filesystem and on a NFS mount.
BUG: 203554
(cherry picked from commit 10ca4ddc8cca49103c2d605a64468788605f3010)
diff --git a/kdecore/io/klockfile_unix.cpp b/kdecore/io/klockfile_unix.cpp
index cd909cf..fa2eda4 100644
--- a/kdecore/io/klockfile_unix.cpp
+++ b/kdecore/io/klockfile_unix.cpp
@@ -1,6 +1,7 @@
/*
This file is part of the KDE libraries
Copyright (c) 2004 Waldo Bastian <bastian@kde.org>
+ Copyright (c) 2011 David Faure <faure@kde.org>
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Library General Public
@@ -35,48 +36,85 @@
#include <QtCore/QDate>
#include <QtCore/QFile>
-#include <QtCore/QTextIStream>
+#include <QTextStream>
#include "krandom.h"
#include "kglobal.h"
#include "kcomponentdata.h"
#include "ktemporaryfile.h"
#include "kde_file.h"
+#include "kfilesystemtype_p.h"
-// TODO: http://www.spinnaker.de/linux/nfs-locking.html
+#include <unistd.h>
+#include <fcntl.h>
+
+// Related reading:
+// http://www.spinnaker.de/linux/nfs-locking.html
+// http://en.wikipedia.org/wiki/File_locking
+// http://apenwarr.ca/log/?m=201012
+
+// Related source code:
+// * lockfile-create, from the lockfile-progs package, uses the link() trick from lockFileWithLink
+// below, so it works over NFS but fails on FAT32 too.
+// * the flock program, which uses flock(LOCK_EX), works on local filesystems (including FAT32),
+// but not NFS.
+// Note about flock: don't unlink, it creates a race. http://world.std.com/~swmcd/steven/tech/flock.html
+
+// fcntl(F_SETLK) is not a good solution.
+// It locks other processes but locking out other threads must be done by hand,
+// and worse, it unlocks when just reading the file in the same process (!).
+// See the apenwarr.ca article above.
+
+// open(O_EXCL) seems to be the best solution for local files (on all filesystems),
+// it only fails over NFS (at least with old NFS servers).
+// See http://www.informit.com/guides/content.aspx?g=cplusplus&seqNum=144
+
+// Conclusion: we use O_EXCL by default, and the link() trick over NFS.
class KLockFile::Private
{
public:
Private(const KComponentData &c)
- : componentData(c)
+ : staleTime(30), // 30 seconds
+ isLocked(false),
+ linkCountSupport(true),
+ m_pid(-1),
+ m_componentData(c)
{
}
- QString file;
+ // The main method
+ KLockFile::LockResult lockFile(KDE_struct_stat &st_buf);
+
+ // Two different implementations
+ KLockFile::LockResult lockFileOExcl(KDE_struct_stat &st_buf);
+ KLockFile::LockResult lockFileWithLink(KDE_struct_stat &st_buf);
+
+ KLockFile::LockResult deleteStaleLock();
+ KLockFile::LockResult deleteStaleLockWithLink();
+
+ void writeIntoLockFile(QFile& file, const KComponentData& componentData);
+ void readLockFile();
+ bool isNfs() const;
+
+ QFile m_file;
+ QString m_fileName;
int staleTime;
bool isLocked;
- bool recoverLock;
bool linkCountSupport;
QTime staleTimer;
KDE_struct_stat statBuf;
- int pid;
- QString hostname;
- QString instance;
- QString lockRecoverFile;
- KComponentData componentData;
+ int m_pid;
+ QString m_hostname;
+ QString m_componentName;
+ KComponentData m_componentData;
};
-// 30 seconds
KLockFile::KLockFile(const QString &file, const KComponentData &componentData)
: d(new Private(componentData))
{
- d->file = file;
- d->staleTime = 30;
- d->isLocked = false;
- d->recoverLock = false;
- d->linkCountSupport = true;
+ d->m_fileName = file;
}
KLockFile::~KLockFile()
@@ -125,31 +163,59 @@ static bool testLinkCountSupport(const QByteArray &fileName)
return (result < 0 || ((result == 0) && (st_buf.st_nlink == 2)));
}
-static KLockFile::LockResult lockFile(const QString &lockFile, KDE_struct_stat &st_buf,
- bool &linkCountSupport, const KComponentData &componentData)
+void KLockFile::Private::writeIntoLockFile(QFile& file, const KComponentData& componentData)
{
- QByteArray lockFileName = QFile::encodeName( lockFile );
- int result = KDE_lstat( lockFileName, &st_buf );
- if (result == 0)
- return KLockFile::LockFail;
-
- KTemporaryFile uniqueFile(componentData);
- uniqueFile.setFileTemplate(lockFile);
- if (!uniqueFile.open())
- return KLockFile::LockError;
- uniqueFile.setPermissions(QFile::ReadUser|QFile::WriteUser|QFile::ReadGroup|QFile::ReadOther);
+ file.setPermissions(QFile::ReadUser|QFile::WriteUser|QFile::ReadGroup|QFile::ReadOther);
char hostname[256];
hostname[0] = 0;
gethostname(hostname, 255);
hostname[255] = 0;
- QString componentName = componentData.componentName();
+ m_hostname = QString::fromLocal8Bit(hostname);
+ m_componentName = componentData.componentName();
+
+ QTextStream stream(&file);
+ m_pid = getpid();
- QTextStream stream(&uniqueFile);
- stream << QString::number(getpid()) << endl
- << componentName << endl
- << hostname << endl;
+ stream << QString::number(m_pid) << endl
+ << m_componentName << endl
+ << m_hostname << endl;
stream.flush();
+}
+
+void KLockFile::Private::readLockFile()
+{
+ m_pid = -1;
+ m_hostname.clear();
+ m_componentName.clear();
+
+ QFile file(m_fileName);
+ if (file.open(QIODevice::ReadOnly))
+ {
+ QTextStream ts(&file);
+ if (!ts.atEnd())
+ m_pid = ts.readLine().toInt();
+ if (!ts.atEnd())
+ m_componentName = ts.readLine();
+ if (!ts.atEnd())
+ m_hostname = ts.readLine();
+ }
+}
+
+KLockFile::LockResult KLockFile::Private::lockFileWithLink(KDE_struct_stat &st_buf)
+{
+ const QByteArray lockFileName = QFile::encodeName( m_fileName );
+ int result = KDE_lstat( lockFileName, &st_buf );
+ if (result == 0) {
+ return KLockFile::LockFail;
+ }
+
+ KTemporaryFile uniqueFile(m_componentData);
+ uniqueFile.setFileTemplate(m_fileName);
+ if (!uniqueFile.open())
+ return KLockFile::LockError;
+
+ writeIntoLockFile(uniqueFile, m_componentData);
QByteArray uniqueName = QFile::encodeName( uniqueFile.fileName() );
@@ -187,20 +253,74 @@ static KLockFile::LockResult lockFile(const QString &lockFile, KDE_struct_stat &
return KLockFile::LockOK;
}
-static KLockFile::LockResult deleteStaleLock(const QString &lockFile, KDE_struct_stat &st_buf, bool &linkCountSupport, const KComponentData &componentData)
+bool KLockFile::Private::isNfs() const
+{
+ const KFileSystemType::Type fsType = KFileSystemType::fileSystemType(m_fileName);
+ return fsType == KFileSystemType::Nfs;
+}
+
+KLockFile::LockResult KLockFile::Private::lockFile(KDE_struct_stat &st_buf)
+{
+ if (isNfs()) {
+ return lockFileWithLink(st_buf);
+ }
+
+ return lockFileOExcl(st_buf);
+}
+
+KLockFile::LockResult KLockFile::Private::lockFileOExcl(KDE_struct_stat &st_buf)
{
- // This is dangerous, we could be deleting a new lock instead of
- // the old stale one, let's be very careful
+ const QByteArray lockFileName = QFile::encodeName( m_fileName );
+
+ int fd = KDE_open(lockFileName.constData(), O_WRONLY | O_CREAT | O_EXCL, 0644);
+ if (fd < 0) {
+ if (errno == EEXIST) {
+ // File already exists
+ return LockFail;
+ } else {
+ return LockError;
+ }
+ }
+ // We hold the lock, continue.
+ if (!m_file.open(fd, QIODevice::WriteOnly)) {
+ return LockError;
+ }
+ writeIntoLockFile(m_file, m_componentData);
+ const int result = KDE_lstat(QFile::encodeName(m_fileName), &st_buf);
+ if (result != 0)
+ return KLockFile::LockError;
+ return KLockFile::LockOK;
+}
- // Create temp file
- KTemporaryFile *ktmpFile = new KTemporaryFile(componentData);
- ktmpFile->setFileTemplate(lockFile);
- if (!ktmpFile->open())
- return KLockFile::LockError;
+KLockFile::LockResult KLockFile::Private::deleteStaleLock()
+{
+ if (isNfs())
+ return deleteStaleLockWithLink();
+
+ // I see no way to prevent the race condition here, where we could
+ // delete a new lock file that another process just got after we
+ // decided the old one was too stale for us too.
+ qWarning("WARNING: deleting stale lockfile %s", qPrintable(m_fileName));
+ QFile::remove(m_fileName);
+ return LockOK;
+}
- QByteArray lckFile = QFile::encodeName(lockFile);
- QByteArray tmpFile = QFile::encodeName(ktmpFile->fileName());
- delete ktmpFile;
+KLockFile::LockResult KLockFile::Private::deleteStaleLockWithLink()
+{
+ // This is dangerous, we could be deleting a new lock instead of
+ // the old stale one, let's be very careful
+
+ // Create temp file
+ KTemporaryFile *ktmpFile = new KTemporaryFile(m_componentData);
+ ktmpFile->setFileTemplate(m_fileName);
+ if (!ktmpFile->open()) {
+ delete ktmpFile;
+ return KLockFile::LockError;
+ }
+
+ const QByteArray lckFile = QFile::encodeName(m_fileName);
+ const QByteArray tmpFile = QFile::encodeName(ktmpFile->fileName());
+ delete ktmpFile;
// link to lock file
if (::link(lckFile, tmpFile) != 0)
@@ -210,7 +330,7 @@ static KLockFile::LockResult deleteStaleLock(const QString &lockFile, KDE_struct
// and if the lock file still matches
KDE_struct_stat st_buf1;
KDE_struct_stat st_buf2;
- memcpy(&st_buf1, &st_buf, sizeof(KDE_struct_stat));
+ memcpy(&st_buf1, &statBuf, sizeof(KDE_struct_stat));
st_buf1.st_nlink++;
if ((KDE_lstat(tmpFile, &st_buf2) == 0) && st_buf1 == st_buf2)
{
@@ -260,8 +380,10 @@ KLockFile::LockResult KLockFile::lock(LockFlags options)
int n = 5;
while(true)
{
- KDE_struct_stat st_buf;
- result = lockFile(d->file, st_buf, d->linkCountSupport, d->componentData);
+ KDE_struct_stat st_buf;
+ // Try to create the lock file
+ result = d->lockFile(st_buf);
+
if (result == KLockFile::LockOK)
{
d->staleTimer = QTime();
@@ -285,25 +407,11 @@ KLockFile::LockResult KLockFile::lock(LockFlags options)
memcpy(&(d->statBuf), &st_buf, sizeof(KDE_struct_stat));
d->staleTimer.start();
- d->pid = -1;
- d->hostname.clear();
- d->instance.clear();
-
- QFile file(d->file);
- if (file.open(QIODevice::ReadOnly))
- {
- QTextStream ts(&file);
- if (!ts.atEnd())
- d->pid = ts.readLine().toInt();
- if (!ts.atEnd())
- d->instance = ts.readLine();
- if (!ts.atEnd())
- d->hostname = ts.readLine();
- }
+ d->readLockFile();
}
bool isStale = false;
- if ((d->pid > 0) && !d->hostname.isEmpty())
+ if ((d->m_pid > 0) && !d->m_hostname.isEmpty())
{
// Check if hostname is us
char hostname[256];
@@ -311,12 +419,12 @@ KLockFile::LockResult KLockFile::lock(LockFlags options)
gethostname(hostname, 255);
hostname[255] = 0;
- if (d->hostname == QLatin1String(hostname))
+ if (d->m_hostname == QLatin1String(hostname))
{
// Check if pid still exists
- int res = ::kill(d->pid, 0);
+ int res = ::kill(d->m_pid, 0);
if ((res == -1) && (errno == ESRCH))
- isStale = true;
+ isStale = true; // pid does not exist
}
}
if (d->staleTimer.elapsed() > (d->staleTime*1000))
@@ -327,7 +435,7 @@ KLockFile::LockResult KLockFile::lock(LockFlags options)
if ((options & ForceFlag) == 0)
return KLockFile::LockStale;
- result = deleteStaleLock(d->file, d->statBuf, d->linkCountSupport, d->componentData);
+ result = d->deleteStaleLock();
if (result == KLockFile::LockOK)
{
@@ -367,17 +475,19 @@ void KLockFile::unlock()
{
if (d->isLocked)
{
- ::unlink(QFile::encodeName(d->file));
+ ::unlink(QFile::encodeName(d->m_fileName));
+ d->m_file.close();
+ d->m_pid = -1;
d->isLocked = false;
}
}
bool KLockFile::getLockInfo(int &pid, QString &hostname, QString &appname)
{
- if (d->pid == -1)
+ if (d->m_pid == -1)
return false;
- pid = d->pid;
- hostname = d->hostname;
- appname = d->instance;
+ pid = d->m_pid;
+ hostname = d->m_hostname;
+ appname = d->m_componentName;
return true;
}
diff --git a/kdecore/tests/CMakeLists.txt b/kdecore/tests/CMakeLists.txt
index 7c3db77..9173746 100644
--- a/kdecore/tests/CMakeLists.txt
+++ b/kdecore/tests/CMakeLists.txt
@@ -97,6 +97,7 @@ KDECORE_EXECUTABLE_TESTS(
dbuscalltest
kmdcodectest
startserviceby
+ klockfile_testlock # helper for klockfiletest
)
########### klocaletest ###############
diff --git a/kdecore/tests/klockfiletest.cpp b/kdecore/tests/klockfiletest.cpp
index 32cdec9..f2c6f91 100644
--- a/kdecore/tests/klockfiletest.cpp
+++ b/kdecore/tests/klockfiletest.cpp
@@ -19,8 +19,12 @@
#include "klockfiletest.h"
#include "klockfiletest.moc"
+#include <kdebug.h>
#include <unistd.h>
+// TODO test locking from two different threads
+//#include <qtconcurrentrun.h>
+
namespace QTest {
template<>
char* toString(const KLockFile::LockResult& result)
@@ -32,6 +36,26 @@ char* toString(const KLockFile::LockResult& result)
}
}
+// Here's how to test file locking on a FAT filesystem, on linux:
+// cd /tmp
+// dd if=/dev/zero of=fatfile count=180000
+// mkfs.vfat -F32 fatfile
+// mkdir -p fatsystem
+// sudo mount -o loop -o uid=$UID fatfile fatsystem
+//
+// static const char *const lockName = "/tmp/fatsystem/klockfiletest.lock";
+//
+// =====================================================================
+//
+// And here's how to test file locking over NFS, on linux:
+// In /etc/exports: /tmp/nfs localhost(rw,sync,no_subtree_check)
+// sudo mount -t nfs localhost:/tmp/nfs /tmp/nfs-mount
+//
+// static const char *const lockName = "/tmp/nfs-mount/klockfiletest.lock";
+//
+// =====================================================================
+//
+
static const char *const lockName = "klockfiletest.lock";
void
@@ -41,19 +65,28 @@ Test_KLockFile::initTestCase()
lockFile = new KLockFile(QLatin1String(lockName));
}
+static KLockFile::LockResult testLockFromProcess(const QString& lockName)
+{
+ const int ret = QProcess::execute("./klockfile_testlock", QStringList() << lockName);
+ return KLockFile::LockResult(ret);
+}
+
void
Test_KLockFile::testLock()
{
QVERIFY(!lockFile->isLocked());
QCOMPARE(lockFile->lock(), KLockFile::LockOK);
QVERIFY(lockFile->isLocked());
-#ifdef Q_WS_WIN
+
+ // Try to lock it again, should fail
KLockFile *lockFile2 = new KLockFile(QLatin1String(lockName));
QVERIFY(!lockFile2->isLocked());
- QCOMPARE(lockFile2->lock(), KLockFile::LockFail);
+ QCOMPARE(lockFile2->lock(KLockFile::NoBlockFlag), KLockFile::LockFail);
QVERIFY(!lockFile2->isLocked());
delete lockFile2;
-#endif
+
+ // Also try from a different process.
+ QCOMPARE(testLockFromProcess(QLatin1String(lockName)), KLockFile::LockFail);
}
void
@@ -102,7 +135,7 @@ Test_KLockFile::testStaleNoBlockFlag()
#else
char hostname[256];
::gethostname(hostname, sizeof(hostname));
-
+
QFile f(lockName);
f.open(QIODevice::WriteOnly);
QTextStream stream(&f);
@@ -113,7 +146,8 @@ Test_KLockFile::testStaleNoBlockFlag()
lockFile = new KLockFile(QLatin1String(lockName));
QVERIFY(!lockFile->isLocked());
QCOMPARE(lockFile->lock(KLockFile::NoBlockFlag), KLockFile::LockStale);
- QTest::ignoreMessage(QtWarningMsg, "WARNING: deleting stale lockfile klockfiletest.lock");
+ QByteArray expectedMsg = QByteArray("WARNING: deleting stale lockfile ") + lockName;
+ QTest::ignoreMessage(QtWarningMsg, expectedMsg);
QCOMPARE(lockFile->lock(KLockFile::NoBlockFlag|KLockFile::ForceFlag), KLockFile::LockOK);
QVERIFY(lockFile->isLocked());