[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kmail-devel
Subject:    Re: Patch: kill running jobs so that kmail exits
From:       David Faure <dfaure () klaralvdalens-datakonsult ! se>
Date:       2004-04-27 20:53:03
Message-ID: 200404272253.03429.dfaure () klaralvdalens-datakonsult ! se
[Download RAW message or body]

On Tuesday 27 April 2004 21:59, Ingo Klöcker wrote:
> You are only handling the case that it's the main window that's closed. 
> You don't handle the case that one of the secondary windows (composer 
> or message window) is closed as last window. At least, I don't see why 
> KMMainWin::queryExit() should be called in this case.
Right, it's not.
The question is what should happen?
Should mail checking happen in the background of the composer window,
or should be stopped when closing the last KMMainWin?
My guess is for the latter - if we agree I can simply move the code to
the destructor or closeEvent and check if there are any visible KMMainWin 
remaining in the static window list.

> Did you try what happens if a user sends a message like in the previous 
> paragraph and uses a sent-mail folder on the IMAP server? Is the sent 
> message still uploaded?
> 
> Furthermore I'm not sure whether longer IMAP operations should be 
> aborted. Why should this be done? Sure, a hanging job should be killed, 
> but all other jobs should keep running (unless KDE is shut down).
>  [...]

Yes, as we discussed on IRC, my first patch killed too much.
I only want to abort safe "check mail" operations (listing folders, listing
messages, retreiving a message when it's not about copying/moving it,
and "checking UID validity"). Aborting "get acl" operations is fine too.

Till told me he had already thought about adding a cancellable property
to jobs. I did that now, both in FolderJobs and IMAP jobDatas.
And instead of calling killAllJobs it calls a new virtual cancelCheckMail(),
so this now affects only IMAP, not POP anymore.
Updated patch attached.

Of course I realize more and more that this would be less necessary if
there was a way to cancel dimap syncs. But I still expect that people
would close the kmail mainwindow without killing the sync first.
Also, this whole thing can help kmail exiting more gracefully when closing
the session and an automatic mailcheck is under way. Better stop as soon as
we can stop in a safe state, then keep the sync going and get killed at a
random point....

-- 
David Faure -- faure@kde.org, dfaure@klaralvdalens-datakonsult.se
Qt/KDE/KOffice developer
Klarälvdalens Datakonsult AB, Platform-independent software solutions

["cancelmailcheck.diff" (text/x-diff)]

Index: cachedimapjob.cpp
===================================================================
RCS file: /home/kde/kdepim/kmail/cachedimapjob.cpp,v
retrieving revision 1.45
diff -u -p -r1.45 cachedimapjob.cpp
--- cachedimapjob.cpp	25 Apr 2004 12:21:33 -0000	1.45
+++ cachedimapjob.cpp	27 Apr 2004 20:26:42 -0000
@@ -172,6 +172,7 @@ void CachedImapJob::listMessages()
   KIO::SimpleJob *job = KIO::get(url, false, false);
   KIO::Scheduler::assignJobToSlave( mAccount->slave(), job );
   ImapAccountBase::jobData jd( url.url(), mFolder->folder() );
+  jd.cancellable = true;
   mAccount->insertJob( job, jd );
   connect( job, SIGNAL( result(KIO::Job *) ),
            this, SLOT( slotListMessagesResult( KIO::Job* ) ) );
@@ -281,6 +282,7 @@ void CachedImapJob::slotGetNextMessage(K
   url.setPath(mFolder->imapPath() + \
QString(";UID=%1;SECTION=BODY.PEEK[]").arg(mfd.uid));  
   ImapAccountBase::jobData jd( url.url(), mFolder->folder() );
+  jd.cancellable = true;
   mMsg->setTransferInProgress(true);
   KIO::SimpleJob *simpleJob = KIO::get(url, false, false);
   KIO::Scheduler::assignJobToSlave(mAccount->slave(), simpleJob);
@@ -536,6 +538,7 @@ void CachedImapJob::checkUidValidity()
   url.setPath( mFolder->imapPath() + ";UID=0:0" );
 
   ImapAccountBase::jobData jd( url.url(), mFolder->folder() );
+  jd.cancellable = true;
 
   KIO::SimpleJob *job = KIO::get( url, false, false );
   KIO::Scheduler::assignJobToSlave( mAccount->slave(), job );
Index: cachedimapjob.h
===================================================================
RCS file: /home/kde/kdepim/kmail/cachedimapjob.h,v
retrieving revision 1.12
diff -u -p -r1.12 cachedimapjob.h
--- cachedimapjob.h	22 Apr 2004 23:33:37 -0000	1.12
+++ cachedimapjob.h	27 Apr 2004 20:26:42 -0000
@@ -94,8 +94,6 @@ public:
 
   virtual ~CachedImapJob();
 
-  void setPassiveDestructor( bool passive ) { mPassiveDestructor = passive; }
-  bool passiveDestructor() { return mPassiveDestructor; }
   void setParentFolder( const KMFolderCachedImap* parent );
 
 protected:
Index: folderjob.cpp
===================================================================
RCS file: /home/kde/kdepim/kmail/folderjob.cpp,v
retrieving revision 1.9
diff -u -p -r1.9 folderjob.cpp
--- folderjob.cpp	20 Apr 2004 17:46:15 -0000	1.9
+++ folderjob.cpp	27 Apr 2004 20:26:42 -0000
@@ -49,6 +49,7 @@ FolderJob::FolderJob( KMMessage *msg, Jo
     mMsgList.append(msg);
     mSets = msg->headerField("X-UID");
   }
+  init();
 }
 
 //----------------------------------------------------------------------------
@@ -59,6 +60,7 @@ FolderJob::FolderJob( const QPtrList<KMM
     mErrorCode( 0 ),
     mPassiveDestructor( false ), mStarted( false )
 {
+  init();
 }
 
 //----------------------------------------------------------------------------
@@ -67,6 +69,22 @@ FolderJob::FolderJob( JobType jt )
     mErrorCode( 0 ),
     mPassiveDestructor( false ), mStarted( false )
 {
+  init();
+}
+
+//----------------------------------------------------------------------------
+void FolderJob::init()
+{
+  switch ( mType ) {
+  case tListMessages:
+  case tGetFolder:
+  case tGetMessage:
+  case tCheckUidValidity:
+    mCancellable = true;
+    break;
+  default:
+    mCancellable = false;
+  }
 }
 
 //----------------------------------------------------------------------------
Index: folderjob.h
===================================================================
RCS file: /home/kde/kdepim/kmail/folderjob.h,v
retrieving revision 1.12
diff -u -p -r1.12 folderjob.h
--- folderjob.h	22 Apr 2004 23:21:12 -0000	1.12
+++ folderjob.h	27 Apr 2004 20:26:42 -0000
@@ -81,6 +81,22 @@ public:
    */
   int error() const { return mErrorCode; }
 
+  /**
+   * @return true if this job can be cancelled, e.g. to exit the application
+   */
+  bool isCancellable() const { return mCancellable; }
+
+  /**
+   * Call this to change the "cancellable" property of this job.
+   * By default, tListMessages, tGetMessage, tGetFolder and tCheckUidValidity
+   * are cancellable, the others are not. But when copying, a non-cancellable
+   * tGetMessage is needed.
+   */
+  void setCancellable( bool b ) { mCancellable = b; }
+
+  void setPassiveDestructor( bool passive ) { mPassiveDestructor = passive; }
+  bool passiveDestructor() { return mPassiveDestructor; }
+
 signals:
   /**
    * Emitted whenever a KMMessage has been completely
@@ -133,6 +149,9 @@ signals:
    */
   void progress( unsigned long bytesDownloaded, unsigned long bytesTotal );
 
+private:
+  void init();
+
 protected:
   /**
    * Has to be reimplemented. It's called by the start() method. Should
@@ -156,6 +175,7 @@ protected:
   //finished() won't be emitted when this is set
   bool                mPassiveDestructor;
   bool                mStarted;
+  bool                mCancellable;
 };
 
 }
Index: imapaccountbase.cpp
===================================================================
RCS file: /home/kde/kdepim/kmail/imapaccountbase.cpp,v
retrieving revision 1.66
diff -u -p -r1.66 imapaccountbase.cpp
--- imapaccountbase.cpp	25 Apr 2004 15:55:39 -0000	1.66
+++ imapaccountbase.cpp	27 Apr 2004 20:26:42 -0000
@@ -328,6 +328,7 @@ namespace KMail {
     jobData jd;
     jd.total = 1; jd.done = 0;
     jd.path = path;
+    jd.cancellable = true;
     // reset for a new listing
     if (reset)
       mHasInbox = false;
@@ -491,6 +492,7 @@ namespace KMail {
     ACLJobs::GetUserRightsJob* job = ACLJobs::getUserRights( mSlave, url );
 
     jobData jd( url.url(), parent );
+    jd.cancellable = true;
     insertJob(job, jd);
 
     connect(job, SIGNAL(result(KIO::Job *)),
@@ -532,6 +534,7 @@ namespace KMail {
 
     ACLJobs::GetACLJob* job = ACLJobs::getACL( mSlave, url );
     jobData jd( url.url(), parent );
+    jd.cancellable = true;
     insertJob(job, jd);
 
     connect(job, SIGNAL(result(KIO::Job *)),
@@ -678,6 +681,34 @@ namespace KMail {
     return !jobsKilled; // jobsKilled==false -> continue==true
   }
 
+  //-----------------------------------------------------------------------------
+  void ImapAccountBase::cancelMailCheck()
+  {
+    QMap<KIO::Job*, jobData>::Iterator it = mapJobData.begin();
+    while ( it != mapJobData.end() ) {
+      kdDebug() << k_funcinfo << "cancellable: " << (*it).cancellable << endl;
+      if ( (*it).cancellable ) {
+        it.key()->kill();
+        QMap<KIO::Job*, jobData>::Iterator rmit = it;
+        ++it;
+        mapJobData.remove( rmit );
+        // We killed a job -> this kills the slave
+        mSlave = 0;
+      } else
+        ++it;
+    }
+
+    for( QPtrListIterator<FolderJob> it( mJobList ); it.current(); ++it ) {
+      if ( it.current()->isCancellable() ) {
+        FolderJob* job = it.current();
+        job->setPassiveDestructor( true );
+        mJobList.remove( job );
+        delete job;
+      } else
+        ++it;
+    }
+  }
+
 
   //-----------------------------------------------------------------------------
   QString ImapAccountBase::jobData::htmlURL() const
Index: imapaccountbase.h
===================================================================
RCS file: /home/kde/kdepim/kmail/imapaccountbase.h,v
retrieving revision 1.38
diff -u -p -r1.38 imapaccountbase.h
--- imapaccountbase.h	25 Apr 2004 15:55:39 -0000	1.38
+++ imapaccountbase.h	27 Apr 2004 20:26:43 -0000
@@ -110,12 +110,12 @@ namespace KMail {
     struct jobData
     {
       // Needed by QMap, don't use
-      jobData() : url(QString::null), parent(0), total(1), done(0), offset(0), \
inboxOnly(false), quiet(false) {} +      jobData() : url(QString::null), parent(0), \
total(1), done(0), offset(0), inboxOnly(false), quiet(false), cancellable(false) {}  \
// Real constructor  jobData( const QString& _url, KMFolder *_parent = 0,
           int _total = 1, int _done = 0, bool _quiet = false, bool _inboxOnly = \
                false )
         : url(_url), parent(_parent), total(_total), done(_done), offset(0),
-      inboxOnly(_inboxOnly), quiet(_quiet)
+      inboxOnly(_inboxOnly), quiet(_quiet), cancellable(false)
       {}
       // Return "url" in a form that can be displayed in HTML (w/o password)
       QString htmlURL() const;
@@ -128,7 +128,7 @@ namespace KMail {
       KMFolder *parent;
       QPtrList<KMMessage> msgList;
       int total, done, offset;
-      bool inboxOnly, quiet, onlySubscribed;
+      bool inboxOnly, quiet, onlySubscribed, cancellable;
     };
 
     typedef QMap<KIO::Job *, jobData>::Iterator JobIterator;
@@ -211,6 +211,11 @@ namespace KMail {
     void killAllJobs( bool disconnectSlave=false ) = 0;
 
     /**
+     * Abort all running mail checks. Used when exiting.
+     */
+    virtual void cancelMailCheck();
+
+    /**
      * Init a new-mail-check for a single folder
      */
     void processNewMailSingleFolder(KMFolder* folder);
Index: kmaccount.h
===================================================================
RCS file: /home/kde/kdepim/kmail/kmaccount.h,v
retrieving revision 1.66
diff -u -p -r1.66 kmaccount.h
--- kmaccount.h	29 Mar 2004 15:33:01 -0000	1.66
+++ kmaccount.h	27 Apr 2004 20:26:43 -0000
@@ -191,6 +191,11 @@ public:
    */
   void checkDone( bool newmails, int newmailCount );
 
+  /**
+   * Abort all running mail checks. Used when exiting.
+   */
+  virtual void cancelMailCheck() {}
+
 signals:
   virtual void finishedCheck(bool newMail);
   virtual void newMailsProcessed(int numberOfNewMails);
Index: kmacctimap.h
===================================================================
RCS file: /home/kde/kdepim/kmail/kmacctimap.h,v
retrieving revision 1.81
diff -u -p -r1.81 kmacctimap.h
--- kmacctimap.h	25 Apr 2004 15:55:39 -0000	1.81
+++ kmacctimap.h	27 Apr 2004 20:26:43 -0000
@@ -64,7 +64,7 @@ public:
   /**
    * Kill the slave if any jobs are active
    */
-  void killAllJobs( bool disconnectSlave=false );
+  virtual void killAllJobs( bool disconnectSlave=false );
 
   /**
    * Set the top level pseudo folder
Index: kmacctmgr.cpp
===================================================================
RCS file: /home/kde/kdepim/kmail/kmacctmgr.cpp,v
retrieving revision 1.111
diff -u -p -r1.111 kmacctmgr.cpp
--- kmacctmgr.cpp	24 Mar 2004 18:51:59 -0000	1.111
+++ kmacctmgr.cpp	27 Apr 2004 20:26:43 -0000
@@ -385,4 +385,12 @@ uint KMAcctMgr::createId()
 }
 
 //-----------------------------------------------------------------------------
+void KMAcctMgr::cancelMailCheck()
+{
+  for ( QPtrListIterator<KMAccount> it(mAcctList) ;
+	it.current() ; ++it ) {
+    it.current()->cancelMailCheck();
+  }
+}
+
 #include "kmacctmgr.moc"
Index: kmacctmgr.h
===================================================================
RCS file: /home/kde/kdepim/kmail/kmacctmgr.h,v
retrieving revision 1.41
diff -u -p -r1.41 kmacctmgr.h
--- kmacctmgr.h	10 Mar 2004 18:41:27 -0000	1.41
+++ kmacctmgr.h	27 Apr 2004 20:26:43 -0000
@@ -65,6 +65,9 @@ public:
   /** Create a new unique ID */
   uint createId();
 
+  /// Called on exit (KMMainWin::queryExit)
+  void cancelMailCheck();
+
 public slots:
   virtual void singleCheckMail(KMAccount *, bool _interactive = true);
   virtual void singleInvalidateIMAPFolders(KMAccount *);
Index: kmbroadcaststatus.cpp
===================================================================
RCS file: /home/kde/kdepim/kmail/kmbroadcaststatus.cpp,v
retrieving revision 1.25
diff -u -p -r1.25 kmbroadcaststatus.cpp
--- kmbroadcaststatus.cpp	11 Jan 2004 21:57:07 -0000	1.25
+++ kmbroadcaststatus.cpp	27 Apr 2004 20:26:43 -0000
@@ -180,6 +180,11 @@ bool KMBroadcastStatus::abortRequested()
   return abortRequested_;
 }
 
+void KMBroadcastStatus::setAbortRequested()
+{
+  abortRequested_ = true;
+}
+
 void KMBroadcastStatus::requestAbort()
 {
   abortRequested_ = true;
Index: kmbroadcaststatus.h
===================================================================
RCS file: /home/kde/kdepim/kmail/kmbroadcaststatus.h,v
retrieving revision 1.9
diff -u -p -r1.9 kmbroadcaststatus.h
--- kmbroadcaststatus.h	27 Apr 2004 11:18:59 -0000	1.9
+++ kmbroadcaststatus.h	27 Apr 2004 20:26:43 -0000
@@ -68,7 +68,12 @@ public:
   bool abortRequested();
   /**  Set the state of the abort requested variable to false */
   void reset();
-
+  /**  Set the state of the abort requested variable to true,
+   * without emitting the signal.
+   * (to let the current jobs run, but stop when possible).
+   * This is only for exiting gracefully, don't use.
+   */
+  void setAbortRequested();
 signals:
 
   /** Emitted when setStatusMsg is called. */
Index: kmcommands.cpp
===================================================================
RCS file: /home/kde/kdepim/kmail/kmcommands.cpp,v
retrieving revision 1.122
diff -u -p -r1.122 kmcommands.cpp
--- kmcommands.cpp	19 Apr 2004 20:47:55 -0000	1.122
+++ kmcommands.cpp	27 Apr 2004 20:26:44 -0000
@@ -237,6 +237,7 @@ void KMCommand::transferSelectedMsgs()
       complete = false;
       KMCommand::mCountJobs++;
       FolderJob *job = thisMsg->parent()->createJob(thisMsg);
+      job->setCancellable( false );
       // emitted when the message was transferred successfully
       connect(job, SIGNAL(messageRetrieved(KMMessage*)),
               this, SLOT(slotMsgTransfered(KMMessage*)));
@@ -678,6 +679,7 @@ void KMSaveMsgCommand::slotSaveDataReq()
       // retrieve Message first
       if (msg->parent()  && !msg->isComplete() ) {
         FolderJob *job = msg->parent()->createJob(msg);
+        job->setCancellable( false );
         connect(job, SIGNAL(messageRetrieved(KMMessage*)),
             this, SLOT(slotMessageRetrievedForSaving(KMMessage*)));
         job->start();
@@ -1496,6 +1498,7 @@ void KMCopyCommand::execute()
       {
         newMsg->setParent(msg->parent());
         FolderJob *job = srcFolder->createJob(newMsg);
+        job->setCancellable( false );
         connect(job, SIGNAL(messageRetrieved(KMMessage*)),
                 mDestFolder, SLOT(reallyAddCopyOfMsg(KMMessage*)));
         // msg musn't be deleted
@@ -2019,6 +2022,7 @@ void KMLoadPartsCommand::start()
       {
         FolderJob *job = curFolder->createJob( mMsg, FolderJob::tGetMessage,
             0, it.current()->msgPart().partSpecifier() );
+        job->setCancellable( false );
         connect( job, SIGNAL(messageUpdated(KMMessage*, QString)),
             this, SLOT(slotPartRetrieved(KMMessage*, QString)) );
         job->start();
Index: kmfoldercachedimap.cpp
===================================================================
RCS file: /home/kde/kdepim/kmail/kmfoldercachedimap.cpp,v
retrieving revision 1.89
diff -u -p -r1.89 kmfoldercachedimap.cpp
--- kmfoldercachedimap.cpp	27 Apr 2004 20:07:22 -0000	1.89
+++ kmfoldercachedimap.cpp	27 Apr 2004 20:26:45 -0000
@@ -47,6 +47,7 @@
 #include "kmfolder.h"
 #include "kmdict.h"
 #include "acljobs.h"
+#include "kmbroadcaststatus.h"
 
 using KMail::CachedImapJob;
 using KMail::ImapAccountBase;
@@ -511,6 +512,15 @@ QString KMFolderCachedImap::state2String
 // the state that should be executed next
 void KMFolderCachedImap::serverSyncInternal()
 {
+  // This is used to stop processing when we're about to exit
+  // and the current job wasn't cancellable.
+  // For user-requested abort, we'll use signalAbortRequested instead.
+  if( KMBroadcastStatus::instance()->abortRequested() ) {
+    resetSyncState();
+    emit folderComplete( this, false );
+    return;
+  }
+
   //kdDebug(5006) << label() << ": " << state2String( mSyncState ) << endl;
   switch( mSyncState ) {
   case SYNC_STATE_INITIAL:
Index: kmfolderimap.cpp
===================================================================
RCS file: /home/kde/kdepim/kmail/kmfolderimap.cpp,v
retrieving revision 1.194
diff -u -p -r1.194 kmfolderimap.cpp
--- kmfolderimap.cpp	25 Apr 2004 14:15:51 -0000	1.194
+++ kmfolderimap.cpp	27 Apr 2004 20:26:46 -0000
@@ -678,6 +678,7 @@ void KMFolderImap::checkValidity()
     return;
   }
   ImapAccountBase::jobData jd( url.url(), folder() );
+  jd.cancellable = true;
   KIO::SimpleJob *job = KIO::get(url, FALSE, FALSE);
   KIO::Scheduler::assignJobToSlave(mAccount->slave(), job);
   mAccount->insertJob(job, jd);
@@ -797,6 +798,7 @@ void KMFolderImap::reallyGetFolder(const
     KIO::SimpleJob *job = KIO::listDir(url, FALSE);
     KIO::Scheduler::assignJobToSlave(mAccount->slave(), job);
     ImapAccountBase::jobData jd( url.url(), folder() );
+    jd.cancellable = true;
     mAccount->insertJob(job, jd);
     connect(job, SIGNAL(result(KIO::Job *)),
             this, SLOT(slotListFolderResult(KIO::Job *)));
@@ -809,6 +811,7 @@ void KMFolderImap::reallyGetFolder(const
     KIO::SimpleJob *newJob = KIO::get(url, FALSE, FALSE);
     KIO::Scheduler::assignJobToSlave(mAccount->slave(), newJob);
     ImapAccountBase::jobData jd( url.url(), folder() );
+    jd.cancellable = true;
     mAccount->insertJob(newJob, jd);
     connect(newJob, SIGNAL(result(KIO::Job *)),
             this, SLOT(slotGetLastMessagesResult(KIO::Job *)));
Index: kmmainwin.cpp
===================================================================
RCS file: /home/kde/kdepim/kmail/kmmainwin.cpp,v
retrieving revision 1.586
diff -u -p -r1.586 kmmainwin.cpp
--- kmmainwin.cpp	21 Mar 2004 22:44:03 -0000	1.586
+++ kmmainwin.cpp	27 Apr 2004 20:26:46 -0000
@@ -9,7 +9,8 @@
 #include "kmsender.h"
 #include "kmbroadcaststatus.h"
 #include "kmglobal.h"
-#include "kapplication.h"
+#include "kmacctmgr.h"
+#include <kapplication.h>
 #include <klocale.h>
 #include <kedittoolbar.h>
 #include <kconfig.h>
@@ -182,3 +183,15 @@ bool KMMainWin::queryClose() {
 
   return true;
 }
+
+bool KMMainWin::queryExit() {
+  // Called when the last visible window (of any sort) was closed
+
+  if ( !kmkernel->haveSystemTrayApplet() ) {
+    // Running KIO jobs prevent kapp from exiting, so we need to kill them
+    // if they are only about checking mail (not important stuff like moving \
messages) +    KMBroadcastStatus::instance()->setAbortRequested();
+    kmkernel->acctMgr()->cancelMailCheck();
+  }
+  return true;
+}
Index: kmmainwin.h
===================================================================
RCS file: /home/kde/kdepim/kmail/kmmainwin.h,v
retrieving revision 1.165
diff -u -p -r1.165 kmmainwin.h
--- kmmainwin.h	15 Apr 2004 13:04:46 -0000	1.165
+++ kmmainwin.h	27 Apr 2004 20:26:46 -0000
@@ -40,6 +40,7 @@ public slots:
 
 protected:
   virtual bool queryClose ();
+  virtual bool queryExit();
 
 protected slots:
   void slotQuit();



_______________________________________________
KMail developers mailing list
KMail-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmail-devel


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic