- 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



git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@40919 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/add-ons/mail_daemon/inbound_protocols/imap/IMAPInboundProtocol.cpp

@@ -77,39 +77,30 @@ DispatcherIMAPListener::FetchEnd()
 const uint32 kMsgStartWatching = '&StW';
 
 
-IMAPMailboxThread::IMAPMailboxThread(IMAPInboundProtocol& protocol,
-	IMAPMailbox& mailbox)
-	:
-	BLooper("IMAPMailboxThread"),
-
-	fProtocol(protocol),
-	fIMAPMailbox(mailbox),
-
-	fIsWatching(false)
+int32
+watch_mailbox(void* data)
 {
-
+	((IMAPMailboxThread*)data)->_Watch();
+	return B_OK;
 }
 
 
-void
-IMAPMailboxThread::MessageReceived(BMessage* message)
+IMAPMailboxThread::IMAPMailboxThread(IMAPInboundProtocol& protocol,
+	IMAPMailbox& mailbox)
+	:
+	fProtocol(protocol),
+	fIMAPMailbox(mailbox),
+
+	fIsWatching(false),
+	fThread(-1)
 {
-	status_t status = B_ERROR;
+	fWatchSyncSem = create_sem(0, "watch sync sem");
+}
 
-	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);
-	}
+IMAPMailboxThread::~IMAPMailboxThread()
+{
+	delete_sem(fWatchSyncSem);
 }
 
 
@@ -131,9 +122,14 @@ IMAPMailboxThread::SyncAndStartWatchingMailbox()
 		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,9 +154,23 @@ IMAPMailboxThread::StopWatchingMailbox()
 		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;
 }
 
 
@@ -192,7 +202,6 @@ MailboxWatcher::StartWatching(const char* mailboxDir)
 }
 
 
-#include <stdio.h>
 void
 MailboxWatcher::MessageReceived(BMessage* message)
 {
@@ -206,7 +215,6 @@ MailboxWatcher::MessageReceived(BMessage* message)
 			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 @@ MailboxWatcher::MessageReceived(BMessage* message)
 
 			case B_ENTRY_MOVED:
 			{
-				printf("entry moved\n");
 				break;
 				entry_ref from;
 				entry_ref to;
@@ -301,7 +308,6 @@ IMAPInboundProtocol::IMAPInboundProtocol(BMailAccountSettings* settings,
 	fIMAPMailbox.SetFetchBodyLimit(bodyLimit);
 
 	fIMAPMailboxThread = new IMAPMailboxThread(*this, fIMAPMailbox);
-	fIMAPMailboxThread->Run();
 
 	// set watch directory
 	fINBOXWatcher = new MailboxWatcher(this);
@@ -316,8 +322,7 @@ IMAPInboundProtocol::~IMAPInboundProtocol()
 	RemoveHandler(fINBOXWatcher);
 	delete fINBOXWatcher;
 
-	fIMAPMailboxThread->Lock();
-	fIMAPMailboxThread->Quit();
+	delete fIMAPMailboxThread;
 }
 
 
@@ -456,7 +461,6 @@ IMAPInboundProtocol::UpdateSettings(const BMessage& settings)
 		delete[] passwd;
 	}
 
-	// restart mailbox's
 	SyncMessages();
 }
 

src/add-ons/mail_daemon/inbound_protocols/imap/IMAPInboundProtocol.h

@@ -41,24 +41,34 @@ private:
 
 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);
-
-			void				MessageReceived(BMessage* message);
+								~IMAPMailboxThread();
 
 			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;
 };
 
 

src/add-ons/mail_daemon/inbound_protocols/imap/IMAPRootInboundProtocol.cpp

@@ -42,7 +42,6 @@ IMAPRootInboundProtocol::Connect(const char* server, const char* username,
 		if (!folders[i].subscribed || folders[i].folder == "INBOX")
 			continue;
 
-
 		IMAPInboundProtocol* inboundProtocol = new IMAPInboundProtocol(
 			&fAccountSettings, folders[i].folder);
 		inboundProtocol->SetMailNotifier(fMailNotifier->Clone());

src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/IMAPMailbox.cpp

@@ -122,11 +122,11 @@ IMAPMailbox::SupportWatching()
 
 
 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 @@ IMAPMailbox::StartWatchingMailbox()
 		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 @@ IMAPMailbox::StartWatchingMailbox()
 		if (status != B_OK)
 			break;
 	}
+
 	atomic_set(&fWatching, 0);
 	return status;
 }

src/add-ons/mail_daemon/inbound_protocols/imap/imap_lib/IMAPMailbox.h

@@ -44,7 +44,7 @@ public:
 			status_t			Sync();
 
 			bool				SupportWatching();
-			status_t			StartWatchingMailbox();
+			status_t			StartWatchingMailbox(sem_id startedSem = -1);
 			status_t			StopWatchingMailbox();
 
 			status_t			CheckMailbox();

src/preferences/mail/DNSQuery.cpp

@@ -95,10 +95,10 @@ status_t
 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 @@ BRawNetBuffer::_Init(const void* buf, size_t size)
 }
 
 
-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 @@ BRawNetBuffer::_ReadStringAt(BString& string, off_t pos)
 		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;
 }
 
 

src/preferences/mail/DNSQuery.h

@@ -42,7 +42,7 @@ public:
 
 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;

src/servers/mail/MailDaemon.cpp

@@ -197,15 +197,15 @@ MailDaemonApp::RefsReceived(BMessage* message)
 			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);
 	}
 }