Finally figured out why the first assert in IsMessageWaiting5() always

worked under R5 but only sometimes for OBOS.  Answer: the OBOS
implementation of BLooper was attempting to lock itself prior to fetching
the message from the queue.  I moved the lock attempt after the fetch and
it worked the same.  I realized that if the system was loaded heavily
enough, the assert might still fail simply because the looper would not had
enough time to get to the fetch (thereby emptying the queue), making the
assert pretty useless.  Given that it was originally there as a probing
test (rather than as an integral part of the unit test), there is no real
need for it anyway, so it is no longer used.


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

src/kits/app/Looper.cpp

@@ -283,11 +283,14 @@ bool BLooper::IsMessageWaiting() const
 
 /**
 	@note:	What we're doing here differs slightly from the R5 implementation.
-			It appears that they probably return count != 0, which gives a false
+			It appears that they probably return count != 0, which gives an
-			true (!) result when port_buffer_size_etc() would block -- which
+			incorrect true result when port_buffer_size_etc() would block --
-			indicates that the port's buffer is empty, so we should return false.
+			which indicates that the port's buffer is empty, so we should return
-			Since we don't actually care about what the error is, we just return
+			false.  Since we don't actually care about what the error is, we
-			count > 0.
+			just return count > 0.  This has some interesting consequences in
+			that we will correctly return 'false' if the port is empty
+			(B_WOULD_BLOCK), whereas R5 will return true.  We call that a bug
+			where I come from. ;)
  */
 	int32 count;
 	do
@@ -779,7 +782,7 @@ status_t BLooper::_PostMessage(BMessage* msg, BHandler* handler,
 
 	if (!err)
 	{
-		err = Messenger.SendMessage(msg, reply_to);
+		err = Messenger.SendMessage(msg, reply_to, 0);
 	}
 
 	return err;
@@ -1062,7 +1065,8 @@ DBG(OUT("BLooper::ConvertToMessage()\n"));
 
 	if (raw != NULL)
 	{
-		if (bmsg->Unflatten((const char*)raw) != B_OK) {
+		if (bmsg->Unflatten((const char*)raw) != B_OK)
+		{
 DBG(OUT("BLooper::ConvertToMessage(): unflattening message failed\n"));
 			delete bmsg;
 			bmsg = NULL;
@@ -1085,6 +1089,7 @@ DBG(OUT("BLooper::task_looper()\n"));
 	while (!fTerminating)
 	{
 DBG(OUT("LOOPER: outer loop\n"));
+		// TODO: timeout determination algo
 		//	Read from message port (how do we determine what the timeout is?)
 DBG(OUT("LOOPER: MessageFromPort()...\n"));
 		BMessage* msg = MessageFromPort();
@@ -1108,25 +1113,31 @@ DBG(OUT("LOOPER: got no message\n"));
 			//	We use zero as our timeout since we know there is stuff there
 			msg = MessageFromPort(0);
 			//	Add messages to queue
-			fQueue->AddMessage(msg);
+			if (msg)
+			{
+				fQueue->AddMessage(msg);
+			}
 		}
 
 		//	loop: As long as there are messages in the queue and the port is
 		//		  empty... and we are not terminating, of course.
 		bool dispatchNextMessage = true;
-		while (!fTerminating && dispatchNextMessage) {
+		while (!fTerminating && dispatchNextMessage)
+		{
 DBG(OUT("LOOPER: inner loop\n"));
-			//	Lock the looper
-			Lock();
-
 			//	Get next message from queue (assign to fLastMessage)
 			fLastMessage = fQueue->NextMessage();
-			if (!fLastMessage) {
+
+			//	Lock the looper
+			Lock();
+			if (!fLastMessage)
+			{
 				// No more messages: Unlock the looper and terminate the
 				// dispatch loop.
 				dispatchNextMessage = false;
-				Unlock();
+			}
-			} else {
+			else
+			{
 DBG(OUT("LOOPER: fLastMessage: 0x%lx: %.4s\n", fLastMessage->what,
 (char*)&fLastMessage->what));
 DBG(fLastMessage->PrintToStream());
@@ -1186,20 +1197,23 @@ DBG(OUT("LOOPER: top_level_filter(): %p\n", handler));
 						DispatchMessage(fLastMessage, handler);
 					}
 				}
+			}
 
-				//	Unlock the looper
+			//	Unlock the looper
-				Unlock();
+			Unlock();
 
-				//	Delete the current message (fLastMessage)
+			//	Delete the current message (fLastMessage)
+			if (fLastMessage)
+			{
 				delete fLastMessage;
 				fLastMessage = NULL;
+			}
 
-				//	Are any messages on the port?
+			//	Are any messages on the port?
-				if (port_count(fMsgPort) > 0)
+			if (port_count(fMsgPort) > 0)
-				{
+			{
-					//	Do outer loop
+				//	Do outer loop
-					dispatchNextMessage = false;
+				dispatchNextMessage = false;
-				}
 			}
 		}
 	}

src/tests/kits/app/blooper/IsMessageWaitingTest.cpp

@@ -54,7 +54,9 @@ void TIsMessageWaitingTest::IsMessageWaiting3()
 #else
 #if 0
 	// Testing to figure out why we get false positives from the R5
-	// implementation of BLooper::IsMessageWaiting().
+	// implementation of BLooper::IsMessageWaiting().  Basically, it tests for
+	// port_buffer_size_etc() != 0 -- which means that return values like
+	// B_WOULD_BLOCK make the function return true, which is just not correct.
 	CPPUNIT_ASSERT(Looper.IsLocked());
 	CPPUNIT_ASSERT(Looper.MessageQueue()->IsEmpty());
 
@@ -89,10 +91,18 @@ void TIsMessageWaitingTest::IsMessageWaiting5()
 	// Prevent a port read
 	Looper->Lock();
 	Looper->PostMessage('1234');
-	CPPUNIT_ASSERT(Looper->MessageQueue()->IsEmpty());
+//	CPPUNIT_ASSERT(Looper->MessageQueue()->IsEmpty());
 	CPPUNIT_ASSERT(Looper->IsMessageWaiting());
 
-	int32 count;
+// Tests to help figure out why the first assert above always worked under R5
+// but only sometimes for OBOS.  Answer: the OBOS implementation of BLooper
+// was attempting to lock itself prior to fetching the message from the queue.
+// I moved the lock attempt after the fetch and it worked the same.  I realized
+// that if the system was loaded heavily enough, the assert might still fail
+// simply because the looper would not had enough time to get to the fetch
+// (thereby emptying the queue), so the assert is no longer used.
+#if 0
+	ssize_t count;
 	do
 	{
 		count = port_buffer_size_etc(_get_looper_port_(Looper), B_TIMEOUT, 0);
@@ -100,10 +110,25 @@ void TIsMessageWaitingTest::IsMessageWaiting5()
 
 	cout << endl << "port_buffer_size_etc: ";
 	if (count < 0)
+	{
 		cout << strerror(count);
+	}
 	else
-		cout << count;
+	{
+		cout << count << endl;
+		char* buffer = new char[count];
+		int32 code;
+		read_port(_get_looper_port_(Looper), &code, (void*)buffer, count);
+		cout << "code: " << code << endl;
+		cout << "buffer: ";
+		for (int32 i = 0; i < count; ++i)
+		{
+			cout << buffer[i];
+		}
+		cout << endl;
+	}
 	cout << endl;
+#endif
 }
 //------------------------------------------------------------------------------
 Test* TIsMessageWaitingTest::Suite()