[PATCH] kdelibs kio::KDirWatch

Flavio Castelli micron at bglug.it
Wed Jul 18 09:12:39 BST 2007


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 <sven at lisa.exp.univie.ac.at>
  * Copyright (C) 2006 Dirk Mueller <mueller at kde.org>
+ * Copyright (C) 2007 Flavio Castelli <flavio.castelli at gmail.com>
  *
  * 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 <ctime>
 
+#include <QThread>
+
 #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<QString,Entry> 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<Entry *> 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 <sven at lisa.exp.univie.ac.at>
    Copyright (C) 2006 Dirk Mueller <mueller at kde.org>
+   Copyright (C) 2007 Flavio Castelli <flavio.castelli at gmail.com>
 
    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); // <e> 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" ~




More information about the kde-core-devel mailing list