From kde-core-devel Wed Jul 18 08:12:39 2007 From: Flavio Castelli Date: Wed, 18 Jul 2007 08:12:39 +0000 To: kde-core-devel Subject: [PATCH] kdelibs kio::KDirWatch Message-Id: <200707181012.39311.micron () bglug ! it> X-MARC-Message: https://marc.info/?l=kde-core-devel&m=118474643005104 Hi, I was adding BSD kqueue support to KDirWatch when, testing my patches, I discovered bug #85989. I tried to fix it and I ended-up with these changes. As you can see from the diff below I have inherited KDirWatchPrivate from QThread, creating a new dedicated thread for file system monitoring. This approach has been adopted also by Trolltech guys into the new QFileSystemWatcher class. I have also made a "small" change making KDirWatch privilege inotify when available. IMHO inotify is better than famd. Most important of all I haven't changed KDirWatch's API, so programs using it wont notice the underling differences. Talking about kqueue support, I've still some bugs to fix and some small problems to solve. I'll commit this new feature as soon as possible. Changes regard only KDirWatchPrivate and take place inside kdirwatch.cpp . Kqueue support will be added in a way similar to inotify one. Since neither ABI nor API will be broken I think the commit can be done also after API freeze. Am I wrong? Feel free to give me your impressions, comments, suggestions and criticism. Also tell me if I can commit these changes the next monday and so close bug #85989. Cheers Flavio Index: kio/kio/kdirwatch_p.h =================================================================== --- kio/kio/kdirwatch_p.h (revision 687368) +++ kio/kio/kdirwatch_p.h (working copy) @@ -6,6 +6,7 @@ * This file is part of the KDE libraries * Copyright (C) 1998 Sven Radej * Copyright (C) 2006 Dirk Mueller + * Copyright (C) 2007 Flavio Castelli * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Library General Public @@ -50,12 +51,14 @@ #include +#include + #define invalid_ctime ((time_t)-1) /* KDirWatchPrivate is a singleton and does the watching * for every KDirWatch instance in the application. */ -class KDirWatchPrivate : public QObject +class KDirWatchPrivate : public QThread { Q_OBJECT public: @@ -106,11 +109,12 @@ #ifdef HAVE_SYS_INOTIFY_H int wd; #endif + }; typedef QMap EntryMap; - KDirWatchPrivate(); + KDirWatchPrivate(QObject* parent = 0); ~KDirWatchPrivate(); void resetList (KDirWatch*,bool); @@ -133,16 +137,20 @@ void ref() { m_ref++; } bool deref() { return ( --m_ref == 0 ); } - static bool isNoisyFile( const char *filename ); + static bool isNoisyFile( const char *filename ); + void init(); + void run(); + void stop(); + public Q_SLOTS: void slotRescan(); void famEventReceived(); // for FAM - void slotActivated(); + void inotifyEventReceived(); // for inotify void slotRemoveDelayed(); public: - QTimer timer; + QTimer* timer; EntryMap m_mapEntries; int freq; @@ -155,7 +163,7 @@ QList removeList; bool rescan_all; - QTimer rescan_timer; + QTimer* rescan_timer; #ifdef HAVE_FAM QSocketNotifier *sn; @@ -173,6 +181,8 @@ bool useINotify(Entry*); #endif + + bool _isReady; }; #endif // KDIRWATCH_P_H Index: kio/kio/kdirwatch.cpp =================================================================== --- kio/kio/kdirwatch.cpp (revision 687368) +++ kio/kio/kdirwatch.cpp (working copy) @@ -2,6 +2,7 @@ /* This file is part of the KDE libraries Copyright (C) 1998 Sven Radej Copyright (C) 2006 Dirk Mueller + Copyright (C) 2007 Flavio Castelli This library is free software; you can redistribute it and/or modify it under the terms of the GNU Library General Public @@ -20,6 +21,8 @@ // CHANGES: +// June 29,2007 - Make KDirWatchPrivate inherit QThread for fixing bug #85989 +// (Flavio Castelli) // Oct 4, 2005 - Inotify support (Dirk Mueller) // Februar 2002 - Add file watching and remote mount check for STAT // Mar 30, 2001 - Native support for Linux dir change notification. @@ -67,7 +70,10 @@ static KDirWatchPrivate* dwp_self = 0; static KDirWatchPrivate* createPrivate() { if (!dwp_self) + { dwp_self = new KDirWatchPrivate; + dwp_self->init(); + } return dwp_self; } @@ -99,28 +105,42 @@ * unmounted. */ -KDirWatchPrivate::KDirWatchPrivate() - : timer(), +KDirWatchPrivate::KDirWatchPrivate(QObject* parent) + : QThread (parent), freq( 3600000 ), // 1 hour as upper bound statEntries( 0 ), m_ref( 0 ), delayRemove( false ), rescan_all( false ), - rescan_timer() + _isReady (false) { - timer.setObjectName( "KDirWatchPrivate::timer" ); - connect (&timer, SIGNAL(timeout()), this, SLOT(slotRescan())); +} +void KDirWatchPrivate::init() +{ KConfigGroup config(KGlobal::config(), QLatin1String("DirWatch")); m_nfsPollInterval = config.readEntry("NFSPollInterval", 5000); m_PollInterval = config.readEntry("PollInterval", 500); + moveToThread (this); + + start(); +} + +void KDirWatchPrivate::run() +{ QString available("Stat"); + // timer for stat + timer = new QTimer (this); + rescan_timer = new QTimer (this); + timer->setObjectName( "KDirWatchPrivate::timer" ); + connect (timer, SIGNAL(timeout()), this, SLOT(slotRescan())); + // used for FAM - rescan_timer.setObjectName( "KDirWatchPrivate::rescan_timer" ); - rescan_timer.setSingleShot( true ); - connect(&rescan_timer, SIGNAL(timeout()), this, SLOT(slotRescan())); + rescan_timer->setObjectName( "KDirWatchPrivate::rescan_timer" ); + rescan_timer->setSingleShot( true ); + connect(rescan_timer, SIGNAL(timeout()), this, SLOT(slotRescan())); #ifdef HAVE_FAM // It's possible that FAM server can't be started @@ -128,9 +148,9 @@ available += ", FAM"; use_fam=true; sn = new QSocketNotifier( FAMCONNECTION_GETFD(&fc), - QSocketNotifier::Read, this); + QSocketNotifier::Read, this); connect( sn, SIGNAL(activated(int)), - this, SLOT(famEventReceived()) ); + this, SLOT(famEventReceived()) ); } else { kDebug(7001) << "Can't use FAM (fam daemon not running?)" << endl; @@ -166,17 +186,26 @@ fcntl(m_inotify_fd, F_SETFD, FD_CLOEXEC); mSn = new QSocketNotifier( m_inotify_fd, QSocketNotifier::Read, this ); - connect( mSn, SIGNAL(activated( int )), this, SLOT( slotActivated() ) ); + connect( mSn, SIGNAL(activated( int )), + this, SLOT( inotifyEventReceived() ) ); } #endif kDebug(7001) << "Available methods: " << available << endl; + + _isReady = true; + exec(); } +void KDirWatchPrivate::stop() +{ + QMetaObject::invokeMethod(this, "quit"); +} + /* This is called on app exit (K_GLOBAL_STATIC) */ KDirWatchPrivate::~KDirWatchPrivate() { - timer.stop(); + timer->stop(); /* remove all entries being watched */ removeEntries(0); @@ -190,11 +219,12 @@ if ( supports_inotify ) ::close( m_inotify_fd ); #endif + } -void KDirWatchPrivate::slotActivated() +void KDirWatchPrivate::inotifyEventReceived() { - + kDebug(7001) << "KDirWatchPrivate::inotifyEventReceived" << endl; #ifdef HAVE_SYS_INOTIFY_H if ( !supports_inotify ) return; @@ -268,8 +298,8 @@ } } - if (!rescan_timer.isActive()) - rescan_timer.start(m_PollInterval); // singleshot + if (!rescan_timer->isActive()) + rescan_timer->start(m_PollInterval); // singleshot break; } @@ -372,7 +402,7 @@ // a reasonable frequency for the global polling timer if (e->freq < freq) { freq = e->freq; - if (timer.isActive()) timer.start(freq); + if (timer->isActive()) timer->start(freq); kDebug(7001) << "Global Poll Freq is now " << freq << " msec" << endl; } } @@ -441,6 +471,8 @@ // setup INotify notification, returns false if not possible bool KDirWatchPrivate::useINotify( Entry* e ) { + kDebug (7001) << "trying to use inotify for monitoring" << endl; + e->wd = 0; e->dirty = false; @@ -466,7 +498,10 @@ if ( ( e->wd = inotify_add_watch( m_inotify_fd, QFile::encodeName( e->path ), mask) ) > 0) + { + kDebug (7001) << "inotify successfully used for monitoring" << endl; return true; + } return false; } @@ -487,7 +522,7 @@ if ( statEntries == 1 ) { // if this was first STAT entry (=timer was stopped) - timer.start(freq); // then start the timer + timer->start(freq); // then start the timer kDebug(7001) << " Started Polling Timer, freq " << freq << endl; } } @@ -507,6 +542,9 @@ void KDirWatchPrivate::addEntry(KDirWatch* instance, const QString& _path, Entry* sub_entry, bool isDir) { + while (!_isReady) // wait until is ready + { } + QString path = _path; if (path.startsWith("/dev/") || (path == "/dev")) return; // Don't even go there. @@ -594,14 +632,18 @@ if ( isNoisyFile( tpath ) ) return; -#if defined(HAVE_FAM) - if (useFAM(e)) return; -#endif + // put inotify check before famd one, otherwise famd will be used + // also when inotify is available. Since inotify works + // better than famd, it is preferred to the last one #if defined(HAVE_SYS_INOTIFY_H) if (useINotify(e)) return; #endif +#if defined(HAVE_FAM) + if (useFAM(e)) return; +#endif + useStat(e); } @@ -609,6 +651,9 @@ void KDirWatchPrivate::removeEntry( KDirWatch* instance, const QString& _path, Entry* sub_entry ) { + while (!_isReady) // wait until is ready + { } + kDebug(7001) << "KDirWatchPrivate::removeEntry for '" << _path << "' sub_entry: " << sub_entry << endl; Entry* e = entry(_path); if (!e) { @@ -669,7 +714,7 @@ if (e->m_mode == StatMode) { statEntries--; if ( statEntries == 0 ) { - timer.stop(); // stop timer if lists are empty + timer->stop(); // stop timer if lists are empty kDebug(7001) << " Stopped Polling Timer" << endl; } } @@ -687,6 +732,9 @@ */ void KDirWatchPrivate::removeEntries( KDirWatch* instance ) { + while (!_isReady) // wait until is ready + { } + int minfreq = 3600000; QStringList pathList; @@ -714,7 +762,7 @@ if (minfreq > freq) { // we can decrease the global polling frequency freq = minfreq; - if (timer.isActive()) timer.start(freq); + if (timer->isActive()) timer->start(freq); kDebug(7001) << "Poll Freq now " << freq << " msec" << endl; } } @@ -969,9 +1017,9 @@ // People can do very long things in the slot connected to dirty(), // like showing a message box. We don't want to keep polling during // that time, otherwise the value of 'delayRemove' will be reset. - bool timerRunning = timer.isActive(); + bool timerRunning = timer->isActive(); if ( timerRunning ) - timer.stop(); + timer->stop(); // We delay deletions of entries this way. // removeDir(), when called in slotDirty(), can cause a crash otherwise @@ -1025,7 +1073,7 @@ if ( timerRunning ) - timer.start(freq); + timer->start(freq); #ifdef HAVE_SYS_INOTIFY_H // Remove watch of parent of new created directories @@ -1057,6 +1105,8 @@ delayRemove = true; + kDebug(7001) << "Fam event received" << endl; + while(use_fam && FAMPending(&fc)) { if (FAMNextEvent(&fc, &fe) == -1) { kWarning(7001) << "FAM connection problem, switching to polling." @@ -1068,12 +1118,12 @@ EntryMap::Iterator it; it = m_mapEntries.begin(); for( ; it != m_mapEntries.end(); ++it ) - if ((*it).m_mode == FAMMode && (*it).m_clients.count()>0) { + if ((*it).m_mode == FAMMode && (*it).m_clients.count()>0) { #ifdef HAVE_SYS_INOTIFY_H - if (useINotify( &(*it) )) continue; + if (useINotify( &(*it) )) continue; #endif - useStat( &(*it) ); - } + useStat( &(*it) ); + } } else checkFAMEvent(&fe); @@ -1084,6 +1134,8 @@ void KDirWatchPrivate::checkFAMEvent(FAMEvent* fe) { + kDebug() << "checkFAMEvent" << endl; + // Don't be too verbose ;-) if ((fe->code == FAMExists) || (fe->code == FAMEndExist) || @@ -1132,8 +1184,8 @@ // Delayed handling. This rechecks changes with own stat calls. e->dirty = true; - if (!rescan_timer.isActive()) - rescan_timer.start(m_PollInterval); // singleshot + if (!rescan_timer->isActive()) + rescan_timer->start(m_PollInterval); // singleshot // needed FAM control actions on FAM events if (e->isDir) @@ -1163,11 +1215,12 @@ QString path = e->path; removeEntry(0,e->path,sub_entry); // can be invalid here!! sub_entry->m_status = Normal; - if (!useFAM(sub_entry)) + if (!useFAM(sub_entry)) { #ifdef HAVE_SYS_INOTIFY_H if (!useINotify(sub_entry )) #endif useStat(sub_entry); + } break; } } @@ -1180,7 +1233,10 @@ } } #else -void KDirWatchPrivate::famEventReceived() {} +void KDirWatchPrivate::famEventReceived() +{ + kWarning (7001) << "Fam event received but FAM isn't supported" << endl; +} #endif @@ -1268,7 +1324,6 @@ } } - // TODO: add watchFiles/recursive support void KDirWatch::addDir( const QString& _path, bool watchFiles, bool recursive) @@ -1388,14 +1443,14 @@ KDirWatch::Method KDirWatch::internalMethod() { +#ifdef HAVE_SYS_INOTIFY_H + if (d->supports_inotify) + return KDirWatch::INotify; +#endif #ifdef HAVE_FAM if (d->use_fam) return KDirWatch::FAM; #endif -#ifdef HAVE_SYS_INOTIFY_H - if (d->supports_inotify) - return KDirWatch::INotify; -#endif return KDirWatch::Stat; } -- |§ micron<- ICQ #118796665 |§ GPG Key: |§ ~ Keyserver: pgp.mit.edu |§ ~ KeyID: 6D632BED ~ "Progress is merely a realisation of utopias" ~