Fix #99561. Use MPSemaphore calls rather than WaitNextEvent to pause CPU under Mac OS X - fixes thread deadlock and improves performance. r=wtc,sr=sfraser,a=dbaron

5 files changed, 138 insertions, 32 deletions

diff --git a/pr/include/md/_macos.h b/pr/include/md/_macos.h
index f1f0f8aa..ccd54a83 100644
--- a/pr/include/md/_macos.h
+++ b/pr/include/md/_macos.h
@@ -681,6 +681,18 @@ extern void LeaveCritialRegion();
 #endif
 
 
+
+/*
+ * CPU Idle support
+ */
+
+extern void InitIdleSemaphore();
+extern void TermIdleSemaphore();
+
+extern void WaitOnIdleSemaphore();
+extern void SignalIdleSemaphore();
+
+
 /*
  * Atomic operations
  */
diff --git a/pr/src/md/mac/macio.c b/pr/src/md/mac/macio.c
index f09f9bc6..b74c864d 100644
--- a/pr/src/md/mac/macio.c
+++ b/pr/src/md/mac/macio.c
@@ -97,15 +97,17 @@ static void AsyncIOCompletion (ExtendedParamBlock *pbAsyncPtr)
     if (_PR_MD_GET_INTSOFF()) {
         thread->md.missedIONotify = PR_TRUE;
         cpu->u.missed[cpu->where] |= _PR_MISSED_IO;
-        return;
     }
+    else {
+        _PR_INTSOFF(is);
 
-    _PR_INTSOFF(is);
+        thread->md.osErrCode = noErr;
+        DoneWaitingOnThisThread(thread);
 
-    thread->md.osErrCode = noErr;
-    DoneWaitingOnThisThread(thread);
+        _PR_FAST_INTSON(is);
+    }
 
-    _PR_FAST_INTSON(is);
+    SignalIdleSemaphore();
 }
 
 void  _MD_SetError(OSErr oserror)
diff --git a/pr/src/md/mac/macsockotpt.c b/pr/src/md/mac/macsockotpt.c
index ef15a56b..f675b61c 100644
--- a/pr/src/md/mac/macsockotpt.c
+++ b/pr/src/md/mac/macsockotpt.c
@@ -173,9 +173,9 @@ static pascal void  DNSNotifierRoutine(void * contextPtr, OTEventCode otEvent, O
 				if (_PR_MD_GET_INTSOFF()) {
 					dnsContext.thread->md.missedIONotify = PR_TRUE;
 					cpu->u.missed[cpu->where] |= _PR_MISSED_IO;
-					return;
 				}
-				DoneWaitingOnThisThread(dnsContext.thread);
+				else
+					DoneWaitingOnThisThread(dnsContext.thread);
 				break;
 		
         case kOTProviderWillClose:
@@ -189,9 +189,10 @@ static pascal void  DNSNotifierRoutine(void * contextPtr, OTEventCode otEvent, O
 				if (_PR_MD_GET_INTSOFF()) {
 					dnsContext.thread->md.missedIONotify = PR_TRUE;
 					cpu->u.missed[cpu->where] |= _PR_MISSED_IO;
-					return;
 				}
-				DoneWaitingOnThisThread(dnsContext.thread);
+				else {
+					DoneWaitingOnThisThread(dnsContext.thread);
+				}
                 break;
 
         default: // or else we don't handle the event
@@ -199,6 +200,8 @@ static pascal void  DNSNotifierRoutine(void * contextPtr, OTEventCode otEvent, O
 		
 	}
 	// or else we don't handle the event
+	
+	SignalIdleSemaphore();
 }
 
 
@@ -296,10 +299,13 @@ WakeUpNotifiedThread(PRThread *thread, OTResult result)
 		if (_PR_MD_GET_INTSOFF()) {
 			thread->md.missedIONotify = PR_TRUE;
 			cpu->u.missed[cpu->where] |= _PR_MISSED_IO;
-			return;
 		}
-		DoneWaitingOnThisThread(thread);
+		else {
+			DoneWaitingOnThisThread(thread);
+		}
 	}
+	
+	SignalIdleSemaphore();
 }
 
 // Notification routine
@@ -1169,10 +1175,13 @@ static pascal void  RawEndpointNotifierRoutine(void * contextPtr, OTEventCode co
 		if (_PR_MD_GET_INTSOFF()) {
 			thread->md.asyncNotifyPending = PR_TRUE;
 			cpu->u.missed[cpu->where] |= _PR_MISSED_IO;
-			return;
 		}
-		DoneWaitingOnThisThread(thread);
+		else {
+			DoneWaitingOnThisThread(thread);
+		}
 	}
+
+	SignalIdleSemaphore();
 }
 
 PRInt32 _MD_accept(PRFileDesc *fd, PRNetAddr *addr, PRUint32 *addrlen, PRIntervalTime timeout)
@@ -1583,7 +1592,6 @@ static PRInt32 SendReceiveDgram(PRFileDesc *fd, void *buf, PRInt32 amount,
     PRThread *me = _PR_MD_CURRENT_THREAD();
     PRInt32 bytesLeft = amount;
     TUnitData dgram;
-    OTResult	result;
 
     PR_ASSERT(flags == 0);
     
@@ -1618,13 +1626,13 @@ static PRInt32 SendReceiveDgram(PRFileDesc *fd, void *buf, PRInt32 amount,
 			fd->secret->md.write.thread = me;
 			fd->secret->md.writeReady = PR_FALSE;				// expect the worst
             err = OTSndUData(endpoint, &dgram);
-			if (result != kOTFlowErr)							// hope for the best
+			if (err != kOTFlowErr)							// hope for the best
 				fd->secret->md.writeReady = PR_TRUE;
 		} else {
 			fd->secret->md.read.thread = me;
 			fd->secret->md.readReady = PR_FALSE;				// expect the worst			
             err = OTRcvUData(endpoint, &dgram, NULL);
-			if (result != kOTNoDataErr)							// hope for the best
+			if (err != kOTNoDataErr)							// hope for the best
 				fd->secret->md.readReady = PR_TRUE;
 		}
 
diff --git a/pr/src/md/mac/macthr.c b/pr/src/md/mac/macthr.c
index 7cd16ccc..8838ef15 100644
--- a/pr/src/md/mac/macthr.c
+++ b/pr/src/md/mac/macthr.c
@@ -226,18 +226,7 @@ void _MD_PauseCPU(PRIntervalTime timeout)
 {
     if (timeout != PR_INTERVAL_NO_WAIT)
     {
-        EventRecord theEvent;
-	
-         /*
-            ** Calling WaitNextEvent() here is suboptimal. This routine should
-            ** pause the process until IO or the timeout occur, yielding time to
-            ** other processes on operating systems that require this (Mac OS classic).
-            ** WaitNextEvent() may incur too much latency, and has other problems,
-            ** such as the potential to drop suspend/resume events, and to handle
-            ** AppleEvents at a time at which we're not prepared to handle them.
-         */
-        (void) WaitNextEvent(nullEvent, &theEvent, 1, NULL);
-	   
+        WaitOnIdleSemaphore(timeout);
         (void) _MD_IOInterrupt();
     }
 }
@@ -528,19 +517,25 @@ void _MD_SetIntsOff(PRInt32 ints)
 #pragma mark -
 #pragma mark CRITICAL REGION SUPPORT
 
+
+static PRBool RunningOnOSX()
+{
+    long    systemVersion;
+    OSErr   err = Gestalt(gestaltSystemVersion, &systemVersion);
+    return (err == noErr) && (systemVersion >= 0x00001000);
+}
+
+
 #if MAC_CRITICAL_REGIONS
 
 MDCriticalRegionID  gCriticalRegion;
 
 void InitCriticalRegion()
 {
-    long        systemVersion;
     OSStatus    err;    
     
     // we only need to do critical region stuff on Mac OS X    
-    err = Gestalt(gestaltSystemVersion, &systemVersion);
-    gUseCriticalRegions = (err == noErr) && (systemVersion >= 0x00001000);
-    
+    gUseCriticalRegions = RunningOnOSX();
     if (!gUseCriticalRegions) return;
     
     err = MD_CriticalRegionCreate(&gCriticalRegion);
@@ -586,3 +581,90 @@ void LeaveCritialRegion()
 
 #endif // MAC_CRITICAL_REGIONS
 
+//##############################################################################
+//##############################################################################
+#pragma mark -
+#pragma mark IDLE SEMAPHORE SUPPORT
+
+/*
+     Since the WaitNextEvent() in _MD_PauseCPU() is causing all sorts of
+     headache under Mac OS X we're going to switch to MPWaitOnSemaphore()
+     which should do what we want
+*/
+
+#if TARGET_CARBON
+PRBool					gUseIdleSemaphore = PR_FALSE;
+MPSemaphoreID			gIdleSemaphore = NULL;
+#endif
+ProcessSerialNumber		gApplicationProcess;
+
+void InitIdleSemaphore()
+{
+    // we only need to do idle semaphore stuff on Mac OS X
+#if TARGET_CARBON
+	gUseIdleSemaphore = RunningOnOSX();
+	if (gUseIdleSemaphore)
+	{
+		OSStatus  err = MPCreateSemaphore(1 /* max value */, 0 /* initial value */, &gIdleSemaphore);
+		PR_ASSERT(err == noErr);
+	}
+	else
+#endif
+	{
+		GetCurrentProcess(&gApplicationProcess);
+	}
+}
+
+void TermIdleSemaphore()
+{
+#if TARGET_CARBON
+	if (gUseIdleSemaphore)
+	{
+		OSStatus  err = MPDeleteSemaphore(gIdleSemaphore);
+		PR_ASSERT(err == noErr);
+		gUseIdleSemaphore = NULL;
+	}
+#endif
+}
+
+
+void WaitOnIdleSemaphore(PRIntervalTime timeout)
+{
+#if TARGET_CARBON
+	if (gUseIdleSemaphore)
+	{
+		OSStatus  err = MPWaitOnSemaphore(gIdleSemaphore, kDurationMillisecond * PR_IntervalToMilliseconds(timeout));
+		PR_ASSERT(err == noErr);
+	}
+	else
+#endif
+	{
+		EventRecord   theEvent;
+		/*
+		** Calling WaitNextEvent() here is suboptimal. This routine should
+		** pause the process until IO or the timeout occur, yielding time to
+		** other processes on operating systems that require this (Mac OS classic).
+		** WaitNextEvent() may incur too much latency, and has other problems,
+		** such as the potential to drop suspend/resume events.
+		*/
+		(void)WaitNextEvent(nullEvent, &theEvent, 1, NULL);
+	}
+}
+
+
+void SignalIdleSemaphore()
+{
+#if TARGET_CARBON
+	if (gUseIdleSemaphore)
+	{
+		// often we won't be waiting on the semaphore here, so ignore any errors
+		(void)MPSignalSemaphore(gIdleSemaphore);
+	}
+	else
+#endif
+	{
+		WakeUpProcess(&gApplicationProcess);
+	}
+}
+
+
diff --git a/pr/src/md/mac/mdmac.c b/pr/src/md/mac/mdmac.c
index f7e9a4fa..cfa33b4f 100644
--- a/pr/src/md/mac/mdmac.c
+++ b/pr/src/md/mac/mdmac.c
@@ -289,6 +289,7 @@ void _MD_EarlyInit()
 	Handle				environmentVariables;
 
 	INIT_CRITICAL_REGION();
+	InitIdleSemaphore();
 
 #if !defined(MAC_NSPR_STANDALONE)
 	// MacintoshInitializeMemory();  Moved to mdmacmem.c: AllocateRawMemory(Size blockSize)
@@ -376,6 +377,7 @@ void CleanupTermProc(void)
 	_MD_StopInterrupts();	// deactive Time Manager task
 
 	CLOSE_OPEN_TRANSPORT();
+	TermIdleSemaphore();
 	TERM_CRITICAL_REGION();
 	
 	__NSTerminate();