Inter-thread SendMessage() bugfixes:

windows/message.c

@@ -670,14 +670,27 @@ static LRESULT MSG_SendMessageInterThread( HQUEUE16 hDestQueue,
     if (QUEUE_AddSMSG(destQ, SM_PENDING_LIST, smsg) == FALSE)
         return 0;
 
-    /* perform task switch and wait for the result */
-    while( (smsg->flags & SMSG_HAVE_RESULT) == 0 )
-    {
     /* force destination task to run next, if 16 bit threads */
-        if (THREAD_IsWin16(THREAD_Current()) && THREAD_IsWin16(destQ->thdb) )
+    if ( THREAD_IsWin16(THREAD_Current()) && THREAD_IsWin16(destQ->thdb) )
         DirectedYield16( destQ->thdb->teb.htask16 );
 
-        if (QUEUE_WaitBits( QS_SMRESULT, timeout ) == 0)
+    /* wait for the result */
+    while ( !(smsg->flags & SMSG_HAVE_RESULT) )
+    {
+        /*
+         * The sequence is crucial to avoid deadlock situations:
+         * - first, we clear the QS_SMRESULT bit
+         * - then, we check the SMSG_HAVE_RESULT bit
+         * - only if this isn't set, we enter the wait state.
+         *
+         * As the receiver first sets the SMSG_HAVE_RESULT and then wakes us,
+         * we are guaranteed that -should we now clear the QS_SMRESULT that
+         * was signalled already by the receiver- we will not start waiting.
+         */
+        QUEUE_ClearWakeBit( queue, QS_SMRESULT );
+
+        if (   !(smsg->flags & SMSG_HAVE_RESULT)
+             && QUEUE_WaitBits( QS_SMRESULT, timeout ) == 0 )
         {
             /* return with timeout */
             SetLastError( 0 );
@@ -685,44 +698,13 @@ static LRESULT MSG_SendMessageInterThread( HQUEUE16 hDestQueue,
             break;
         }
 
-        if (! (smsg->flags & SMSG_HAVE_RESULT) )
-        {
-            /* not supposed to happen */
-            ERR(sendmsg, "SMSG_HAVE_RESULT not set: smsg->flags=%x\n", smsg->flags);
-            QUEUE_ClearWakeBit( queue, QS_SMRESULT );
-      }
-        else
+        if ( smsg->flags & SMSG_HAVE_RESULT )
         {
             *pRes = smsg->lResult;
             TRACE(sendmsg,"smResult = %08x\n", (unsigned)*pRes );
         }
     }
 
-    /*
-     * Warning: This one looks like a hack but it has a very good reason
-     * for existing. It will become obsolete once an input task/thread is
-     * implemented
-     *
-     * Normally, once a send message operation is complete, you want to 
-     * clear the wake bit QS_SMRESULT for this queue. However, because of
-     * the way the 16 bit threads are scheduled, the EVENT_WaitNetEvent
-     * method might be sending messages using the same message queue as the
-     * one used for this send message. Since the recipient of that other
-     * send message is in another thread, it is totally possible that a
-     * send message operation that occured in time before this one
-     * has completed it's work before this one does.
-     * If we clear the wake bit without checking and that operation has 
-     * completed, the send message operation waiting in the call stack 
-     * of the current thread will wait forever.
-     */
-    EnterCriticalSection(&queue->cSection);
-
-    if ( (smsg->nextProcessing==0) ||
-	 (( smsg->nextProcessing->flags & SMSG_HAVE_RESULT) == 0 ) )
-      QUEUE_ClearWakeBit( queue, QS_SMRESULT );
-
-    LeaveCriticalSection(&queue->cSection);
-
     /* remove the smsg from the processingg list of the source queue */
     QUEUE_RemoveSMSG( queue, SM_PROCESSING_LIST, smsg );
 
@@ -774,30 +756,17 @@ BOOL WINAPI ReplyMessage( LRESULT result )
 
     TRACE(sendmsg,"ReplyMessage, queue %04x\n", queue->self);
 
-    while ((smsg = queue->smWaiting) != 0)
-    {
-        senderQ = (MESSAGEQUEUE*)QUEUE_Lock( smsg->hSrcQueue );
-        if ( !senderQ )
+    if (    !(smsg = queue->smWaiting)
+         || !(senderQ = QUEUE_Lock( smsg->hSrcQueue )) )
         goto ReplyMessageEnd;
 
-        /* if message has already been reply, continue the loop of receving
-         message */
-        if ( smsg->flags & SMSG_ALREADY_REPLIED )
-            goto ReplyMessageDone;
-
-        /* if send message pending, processed it */
-      if( queue->wakeBits & QS_SENDMESSAGE )
+    if ( !(smsg->flags & SMSG_ALREADY_REPLIED) )
     {
-            /* Note: QUEUE_ReceiveMessage() and ReplyMessage call each other */
-	QUEUE_ReceiveMessage( queue );
-        QUEUE_Unlock( senderQ );
-	continue; /* ReceiveMessage() already called us */
-      }
-        break;   /* message to reply is in smsg */
-    }
+        /* This is the first reply, so pass result to sender */
 
-    if ( !smsg )
-        goto ReplyMessageEnd;
+        TRACE( sendmsg,"\trpm: smResult = %08lx\n", (long) result );
+
+        EnterCriticalSection(&senderQ->cSection);
         
         smsg->lResult = result;
         smsg->flags |= SMSG_ALREADY_REPLIED;
@@ -807,24 +776,20 @@ BOOL WINAPI ReplyMessage( LRESULT result )
         if ( !(smsg->flags & SMSG_SENDING_REPLY) )
             smsg->flags |= SMSG_EARLY_REPLY;
 
-    TRACE( sendmsg,"\trpm: smResult = %08lx\n", (long) result );
-
-    EnterCriticalSection(&senderQ->cSection);
-
         smsg->flags |= SMSG_HAVE_RESULT;
 
+        LeaveCriticalSection(&senderQ->cSection);
+
         /* tell the sending task that its reply is ready */
         QUEUE_SetWakeBit( senderQ, QS_SMRESULT );
 
-    LeaveCriticalSection(&senderQ->cSection);
-
         /* switch directly to sending task (16 bit thread only) */
         if ( THREAD_IsWin16( THREAD_Current() ) && THREAD_IsWin16( senderQ->thdb ) )
             DirectedYield16( senderQ->thdb->teb.htask16 );
 
         ret = TRUE;
+    }
     
-ReplyMessageDone:
     if (smsg->flags & SMSG_SENDING_REPLY)
     {
         /* remove msg from the waiting list, since this  is the last

windows/queue.c

@@ -750,15 +750,22 @@ BOOL QUEUE_AddSMSG( MESSAGEQUEUE *queue, int list, SMSG *smsg )
             break;
             
         case SM_PENDING_LIST:
+        {
             /* make it thread safe, could be accessed by the sender and
              receiver thread */
+            SMSG **prev;
 
             EnterCriticalSection( &queue->cSection );
-            smsg->nextPending = queue->smPending;
-            queue->smPending = smsg;
-            QUEUE_SetWakeBit( queue, QS_SENDMESSAGE );
+            smsg->nextPending = NULL;
+            prev = &queue->smPending;
+            while ( *prev )
+                prev = &(*prev)->nextPending;
+            *prev = smsg;
             LeaveCriticalSection( &queue->cSection );
+
+            QUEUE_SetWakeBit( queue, QS_SENDMESSAGE );
             break;
+        }
 
         default:
             WARN(sendmsg, "Invalid list: %d", list);