[haiku-commits] r40919 - in haiku/trunk/src: add-ons/mail_daemon/inbound_protocols/imap add-ons/mail_daemon/inbound_protocols/imap/imap_lib preferences/mail servers/mail

  • From: clemens.zeidler@xxxxxxxxxxxxxx
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Fri, 11 Mar 2011 21:13:55 +0100 (CET)

Author: czeidler
Date: 2011-03-11 21:13:54 +0100 (Fri, 11 Mar 2011)
New Revision: 40919
Changeset: http://dev.haiku-os.org/changeset/40919

Modified:
   
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPInboundProtocol.cpp
   
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPInboundProtocol.h
   
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPRootInboundProtocol.cpp
   
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/IMAPMailbox.cpp
   
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/IMAPMailbox.h
   haiku/trunk/src/preferences/mail/DNSQuery.cpp
   haiku/trunk/src/preferences/mail/DNSQuery.h
   haiku/trunk/src/servers/mail/MailDaemon.cpp
Log:
- Fix race condition when start watching a mailbox and directly afterwards stop 
watching it. A BLooper was not suitable to synchronise start and stop watching. 
Wait till the IDLE command is send before returning the 
SyncAndStartWatchingMailbox method now. That ensures that a later 
StopWatchingMailbox call find the maibox in an expected watching state.

There is one thread (BLooper) to handle new commands and one watcher thread 
which is just listening at the server port for updates. The race condition 
occurred for example when a sync/watching and a fetch body message are send to 
the looper. The sync message just triggered the IDLE command in the watcher 
thread. In the meantime the fetch body command send a DONE command, because the 
IDLE command has not be send at this time the watcher keeps watching. 

- fix int32 -> ssize_t thanks Axel and Stippi
- clean up



Modified: 
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPInboundProtocol.cpp
===================================================================
--- 
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPInboundProtocol.cpp
  2011-03-11 19:52:15 UTC (rev 40918)
+++ 
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPInboundProtocol.cpp
  2011-03-11 20:13:54 UTC (rev 40919)
@@ -77,39 +77,30 @@
 const uint32 kMsgStartWatching = '&StW';
 
 
+int32
+watch_mailbox(void* data)
+{
+       ((IMAPMailboxThread*)data)->_Watch();
+       return B_OK;
+}
+
+
 IMAPMailboxThread::IMAPMailboxThread(IMAPInboundProtocol& protocol,
        IMAPMailbox& mailbox)
        :
-       BLooper("IMAPMailboxThread"),
-
        fProtocol(protocol),
        fIMAPMailbox(mailbox),
 
-       fIsWatching(false)
+       fIsWatching(false),
+       fThread(-1)
 {
-
+       fWatchSyncSem = create_sem(0, "watch sync sem");
 }
 
 
-void
-IMAPMailboxThread::MessageReceived(BMessage* message)
+IMAPMailboxThread::~IMAPMailboxThread()
 {
-       status_t status = B_ERROR;
-
-       switch (message->what) {
-               case kMsgStartWatching:
-                       status = fIMAPMailbox.StartWatchingMailbox();
-                       if (status != B_OK)
-                               fProtocol.Disconnect();
-
-                       fLock.Lock();
-                       fIsWatching = false;
-                       fLock.Unlock();
-                       break;
-
-               default:
-                       BLooper::MessageReceived(message);
-       }
+       delete_sem(fWatchSyncSem);
 }
 
 
@@ -131,9 +122,14 @@
                BAutolock autolock(fLock);
                if (fIsWatching)
                        return B_OK;
+               fThread = spawn_thread(watch_mailbox, "IMAPMailboxThread",
+                       B_LOW_PRIORITY, this);
+               if (resume_thread(fThread) != B_OK) {
+                       fThread = -1;
+                       return B_ERROR;
+               }
+               acquire_sem(fWatchSyncSem);
                fIsWatching = true;
-               autolock.Unlock();
-               PostMessage(kMsgStartWatching);
        } else {
                status_t status = fIMAPMailbox.CheckMailbox();
                // if we lost connection reconnect and try again
@@ -158,12 +154,26 @@
                return status;
 
        // wait till watching stopped
-       const uint32 kMsgNoMeaning = '&NME';
-       BMessage reply;
-       return BMessenger(this).SendMessage(kMsgNoMeaning, &reply);
+       status_t exitCode;
+       return wait_for_thread(fThread, &exitCode);
 }
 
 
+void
+IMAPMailboxThread::_Watch()
+{
+       status_t status = fIMAPMailbox.StartWatchingMailbox(fWatchSyncSem);
+       if (status != B_OK)
+               fProtocol.Disconnect();
+
+       fLock.Lock();
+       fIsWatching = false;
+       fLock.Unlock();
+
+       fThread = -1;
+}
+
+
 // #pragma mark -
 
 
@@ -192,7 +202,6 @@
 }
 
 
-#include <stdio.h>
 void
 MailboxWatcher::MessageReceived(BMessage* message)
 {
@@ -206,7 +215,6 @@
                        break;
                switch (opcode) {
                        case B_ENTRY_CREATED:
-                               printf("entry created\n");
                                break;
                                message->FindInt32("device", &ref.device);
                                message->FindInt64("directory", &ref.directory);
@@ -225,7 +233,6 @@
 
                        case B_ENTRY_MOVED:
                        {
-                               printf("entry moved\n");
                                break;
                                entry_ref from;
                                entry_ref to;
@@ -301,7 +308,6 @@
        fIMAPMailbox.SetFetchBodyLimit(bodyLimit);
 
        fIMAPMailboxThread = new IMAPMailboxThread(*this, fIMAPMailbox);
-       fIMAPMailboxThread->Run();
 
        // set watch directory
        fINBOXWatcher = new MailboxWatcher(this);
@@ -316,8 +322,7 @@
        RemoveHandler(fINBOXWatcher);
        delete fINBOXWatcher;
 
-       fIMAPMailboxThread->Lock();
-       fIMAPMailboxThread->Quit();
+       delete fIMAPMailboxThread;
 }
 
 
@@ -456,7 +461,6 @@
                delete[] passwd;
        }
 
-       // restart mailbox's
        SyncMessages();
 }
 

Modified: 
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPInboundProtocol.h
===================================================================
--- 
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPInboundProtocol.h
    2011-03-11 19:52:15 UTC (rev 40918)
+++ 
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPInboundProtocol.h
    2011-03-11 20:13:54 UTC (rev 40919)
@@ -41,24 +41,34 @@
 
 class IMAPInboundProtocol;
 
+
+int32 watch_mailbox(void* data);
+
+
 /*! Just wait for a IDLE (watching) IMAP response in this thread. */
-class IMAPMailboxThread : public BLooper {
+class IMAPMailboxThread {
 public:
                                                                
IMAPMailboxThread(IMAPInboundProtocol& protocol,
                                                                        
IMAPMailbox& mailbox);
+                                                               
~IMAPMailboxThread();
 
-                       void                            
MessageReceived(BMessage* message);
-
                        bool                            IsWatching();
                        status_t                        
SyncAndStartWatchingMailbox();
                        status_t                        StopWatchingMailbox();
 
 private:
+                       void                            _Watch();
+
+       friend  int32 watch_mailbox(void* data);
+
                        IMAPInboundProtocol&    fProtocol;
                        IMAPMailbox&            fIMAPMailbox;
 
                        BLocker                         fLock;
                        bool                            fIsWatching;
+
+                       thread_id                       fThread;
+                       sem_id                          fWatchSyncSem;
 };
 
 

Modified: 
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPRootInboundProtocol.cpp
===================================================================
--- 
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPRootInboundProtocol.cpp
      2011-03-11 19:52:15 UTC (rev 40918)
+++ 
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/IMAPRootInboundProtocol.cpp
      2011-03-11 20:13:54 UTC (rev 40919)
@@ -42,7 +42,6 @@
                if (!folders[i].subscribed || folders[i].folder == "INBOX")
                        continue;
 
-
                IMAPInboundProtocol* inboundProtocol = new IMAPInboundProtocol(
                        &fAccountSettings, folders[i].folder);
                inboundProtocol->SetMailNotifier(fMailNotifier->Clone());

Modified: 
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/IMAPMailbox.cpp
===================================================================
--- 
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/IMAPMailbox.cpp
 2011-03-11 19:52:15 UTC (rev 40918)
+++ 
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/IMAPMailbox.cpp
 2011-03-11 20:13:54 UTC (rev 40919)
@@ -122,11 +122,11 @@
 
 
 status_t
-IMAPMailbox::StartWatchingMailbox()
+IMAPMailbox::StartWatchingMailbox(sem_id startedSem)
 {
-       //TODO set it when we actually watching
        atomic_set(&fWatching, 1);
 
+       bool firstIDLE = true;
        // refresh every 29 min
        bigtime_t timeout = 1000 * 1000 * 60 * 29; // 29 min
        status_t status;
@@ -134,8 +134,13 @@
                int32 commandId = NextCommandId();
                TRACE("IDLE ...\n");
                status = SendCommand("IDLE", commandId);
+               if (firstIDLE) {
+                       release_sem(startedSem);
+                       firstIDLE = false;
+               }
                if (status != B_OK)
                        break;
+
                status = HandleResponse(commandId, timeout, false);
                ProcessAfterQuacks(kIMAP4ClientTimeout);
 
@@ -157,6 +162,7 @@
                if (status != B_OK)
                        break;
        }
+
        atomic_set(&fWatching, 0);
        return status;
 }

Modified: 
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/IMAPMailbox.h
===================================================================
--- 
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/IMAPMailbox.h
   2011-03-11 19:52:15 UTC (rev 40918)
+++ 
haiku/trunk/src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/IMAPMailbox.h
   2011-03-11 20:13:54 UTC (rev 40919)
@@ -44,7 +44,7 @@
                        status_t                        Sync();
 
                        bool                            SupportWatching();
-                       status_t                        StartWatchingMailbox();
+                       status_t                        
StartWatchingMailbox(sem_id startedSem = -1);
                        status_t                        StopWatchingMailbox();
 
                        status_t                        CheckMailbox();

Modified: haiku/trunk/src/preferences/mail/DNSQuery.cpp
===================================================================
--- haiku/trunk/src/preferences/mail/DNSQuery.cpp       2011-03-11 19:52:15 UTC 
(rev 40918)
+++ haiku/trunk/src/preferences/mail/DNSQuery.cpp       2011-03-11 20:13:54 UTC 
(rev 40919)
@@ -95,10 +95,10 @@
 BRawNetBuffer::ReadString(BString& string)
 {
        string = "";
-       int32 read = _ReadStringAt(string, fReadPosition);
-       if (read < 0)
+       ssize_t bytesRead = _ReadStringAt(string, fReadPosition);
+       if (bytesRead < 0)
                return B_ERROR;
-       fReadPosition += read;
+       fReadPosition += bytesRead;
        return B_OK;
 }
 
@@ -122,13 +122,13 @@
 }
 
 
-int32
+ssize_t
 BRawNetBuffer::_ReadStringAt(BString& string, off_t pos)
 {
        if (pos >= fBuffer.BufferLength())
                return -1;
 
-       int32 readed = 0;
+       ssize_t bytesRead = 0;
        char* buffer = (char*)fBuffer.Buffer();
        buffer = &buffer[pos];
        // if the string is compressed we have to follow the links to the
@@ -137,17 +137,17 @@
                if (uint8(*buffer) == 192) {
                        // found a pointer mark
                        buffer++;
-                       readed++;
+                       bytesRead++;
                        off_t subPos = uint8(*buffer);
                        _ReadStringAt(string, subPos);
                        break;
                }
                string.Append(buffer, 1);
                buffer++;
-               readed++;
+               bytesRead++;
        }
-       readed++;
-       return readed;
+       bytesRead++;
+       return bytesRead;
 }
 
 

Modified: haiku/trunk/src/preferences/mail/DNSQuery.h
===================================================================
--- haiku/trunk/src/preferences/mail/DNSQuery.h 2011-03-11 19:52:15 UTC (rev 
40918)
+++ haiku/trunk/src/preferences/mail/DNSQuery.h 2011-03-11 20:13:54 UTC (rev 
40919)
@@ -42,7 +42,7 @@
 
 private:
                void                    _Init(const void* buf, size_t size);
-               int32                   _ReadStringAt(BString& string, off_t 
pos);
+               ssize_t                 _ReadStringAt(BString& string, off_t 
pos);
 
                off_t                   fWritePosition;
                off_t                   fReadPosition;

Modified: haiku/trunk/src/servers/mail/MailDaemon.cpp
===================================================================
--- haiku/trunk/src/servers/mail/MailDaemon.cpp 2011-03-11 19:52:15 UTC (rev 
40918)
+++ haiku/trunk/src/servers/mail/MailDaemon.cpp 2011-03-11 20:13:54 UTC (rev 
40919)
@@ -197,15 +197,15 @@
                        sizeof(account)) < 0)
                        continue;
 
-               InboundProtocolThread* protocol = _FindInboundProtocol(account);
-               if (!protocol)
+               InboundProtocolThread* protocolThread = 
_FindInboundProtocol(account);
+               if (!protocolThread)
                        continue;
 
                BMessenger target;
                BMessenger* messenger = &target;
                if (message->FindMessenger("target", &target) != B_OK)
                        messenger = NULL;
-               protocol->FetchBody(ref, messenger);
+               protocolThread->FetchBody(ref, messenger);
        }
 }
 


Other related posts:

  • » [haiku-commits] r40919 - in haiku/trunk/src: add-ons/mail_daemon/inbound_protocols/imap add-ons/mail_daemon/inbound_protocols/imap/imap_lib preferences/mail servers/mail - clemens . zeidler