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());