Rebuild GEvent.

It should be faster, have less contention problems, use less memory and allow reentrancy from a callback handler.

2 files changed, 82 insertions, 37 deletions

diff --git a/src/gevent/gevent.c b/src/gevent/gevent.c
index 779f63a0..f1dab064 100644
--- a/src/gevent/gevent.c
+++ b/src/gevent/gevent.c
@@ -22,26 +22,40 @@
 	#define GEVENT_ASSERT(x)
 #endif
 
+/* Flags in the listener structure */
+#define GLISTENER_EVENTBUSY			0x0001			// The event buffer is busy
+#define GLISTENER_WAITING			0x0002			// The listener is waiting for a signal
+#define GLISTENER_WITHSOURCE		0x0004			// The listener is being looked at by a source for a possible event
+
 /* This mutex protects access to our tables */
 static gfxMutex	geventMutex;
 
 /* Our table of listener/source pairs */
 static GSourceListener		Assignments[GEVENT_MAX_SOURCE_LISTENERS];
 
+/* Send an exit event if possible. */
+/* We already have the geventMutex */
+static void doExitEvent(GListener *pl) {
+	// Don't do the exit if someone else currently has the event lock
+	if ((pl->flags & (GLISTENER_WAITING|GLISTENER_EVENTBUSY)) == GLISTENER_WAITING) {
+		pl->flags |= GLISTENER_EVENTBUSY;							// Event buffer is in use
+		pl->event.type = GEVENT_EXIT;								// Set up the EXIT event
+		pl->flags &= ~GLISTENER_WAITING;							// Wake up the listener (with data)
+		gfxSemSignal(&pl->waitqueue);
+	}
+}
+
 /* Loop through the assignment table deleting this listener/source pair. */
 /*	Null is treated as a wildcard. */
+/* We already have the geventMutex */
 static void deleteAssignments(GListener *pl, GSourceHandle gsh) {
 	GSourceListener *psl;
 
 	for(psl = Assignments; psl < Assignments+GEVENT_MAX_SOURCE_LISTENERS; psl++) {
 		if ((!pl || psl->pListener == pl) && (!gsh || psl->pSource == gsh)) {
-			if (gfxSemCounter(&psl->pListener->waitqueue) < 0) {
-				gfxSemWait(&psl->pListener->eventlock, TIME_INFINITE);	// Obtain the buffer lock
-				psl->pListener->event.type = GEVENT_EXIT;				// Set up the EXIT event
-				gfxSemSignal(&psl->pListener->waitqueue);				// Wake up the listener
-				gfxSemSignal(&psl->pListener->eventlock);				// Release the buffer lock
-			}
+			doExitEvent(psl->pListener);
 			psl->pListener = 0;
+			psl->pSource = 0;
 		}
 	}
 }
@@ -58,9 +72,9 @@ void _geventDeinit(void)
 
 void geventListenerInit(GListener *pl) {
 	gfxSemInit(&pl->waitqueue, 0, MAX_SEMAPHORE_COUNT);		// Next wait'er will block
-	gfxSemInit(&pl->eventlock, 1, 1);						// Only one thread at a time looking at the event buffer
 	pl->callback = 0;										// No callback active
 	pl->event.type = GEVENT_NULL;							// Always safety
+	pl->flags = 0;
 }
 
 bool_t geventAttachSource(GListener *pl, GSourceHandle gsh, unsigned flags) {
@@ -80,9 +94,7 @@ bool_t geventAttachSource(GListener *pl, GSourceHandle gsh, unsigned flags) {
 		
 		if (pl == psl->pListener && gsh == psl->pSource) {
 			// Just update the flags
-			gfxSemWait(&pl->eventlock, TIME_INFINITE);		// Safety first - just in case a source is using it
 			psl->listenflags = flags;
-			gfxSemSignal(&pl->eventlock);			// Release this lock
 			gfxMutexExit(&geventMutex);
 			return TRUE;
 		}
@@ -106,33 +118,37 @@ void geventDetachSource(GListener *pl, GSourceHandle gsh) {
 	if (pl) {
 		gfxMutexEnter(&geventMutex);
 		deleteAssignments(pl, gsh);
-		if (!gsh && gfxSemCounter(&pl->waitqueue) < 0) {
-			gfxSemWait(&pl->eventlock, TIME_INFINITE);		// Obtain the buffer lock
-			pl->event.type = GEVENT_EXIT;					// Set up the EXIT event
-			gfxSemSignal(&pl->waitqueue);					// Wake up the listener
-			gfxSemSignal(&pl->eventlock);					// Release the buffer lock
-		}
+		if (!gsh)
+			doExitEvent(pl);
 		gfxMutexExit(&geventMutex);
 	}
 }
 
 GEvent *geventEventWait(GListener *pl, delaytime_t timeout) {
-	if (pl->callback || gfxSemCounter(&pl->waitqueue) < 0)
+	gfxMutexEnter(&geventMutex);
+	// Don't allow waiting if we are on callbacks or if there is already a thread waiting
+	if (pl->callback || (pl->flags & GLISTENER_WAITING)) {
+		gfxMutexExit(&geventMutex);
 		return 0;
+	}
+	pl->flags &= ~GLISTENER_EVENTBUSY;				// Event buffer is definitely not busy
+	pl->flags |= GLISTENER_WAITING;					// We will now be waiting on the thread
+	gfxMutexExit(&geventMutex);
 	return gfxSemWait(&pl->waitqueue, timeout) ? &pl->event : 0;
 }
 
+void geventEventComplete(GListener *pl) {
+	pl->flags &= ~GLISTENER_EVENTBUSY;
+}
+
 void geventRegisterCallback(GListener *pl, GEventCallbackFn fn, void *param) {
 	if (pl) {
 		gfxMutexEnter(&geventMutex);
-		gfxSemWait(&pl->eventlock, TIME_INFINITE);			// Obtain the buffer lock
-		pl->param = param;									// Set the param
-		pl->callback = fn;									// Set the callback function
-		if (gfxSemCounter(&pl->waitqueue) < 0) {
-			pl->event.type = GEVENT_EXIT;			// Set up the EXIT event
-			gfxSemSignal(&pl->waitqueue);			// Wake up the listener
-		}
-		gfxSemSignal(&pl->eventlock);				// Release the buffer lock
+		doExitEvent(pl);
+		pl->param = param;						// Set the param
+		pl->callback = fn;						// Set the callback function
+		if (fn)
+			pl->flags &= ~GLISTENER_EVENTBUSY;	// The event buffer is immediately available
 		gfxMutexExit(&geventMutex);
 	}
 }
@@ -146,14 +162,13 @@ GSourceListener *geventGetSourceListener(GSourceHandle gsh, GSourceListener *las
 
 	gfxMutexEnter(&geventMutex);
 
-	// Unlock the last listener event buffer
-	if (lastlr)
-		gfxSemSignal(&lastlr->pListener->eventlock);
+	// Unlock the last listener event buffer if it wasn't used.
+	if (lastlr && lastlr->pListener && (lastlr->pListener->flags & GLISTENER_WITHSOURCE))
+		lastlr->pListener->flags &= ~(GLISTENER_WITHSOURCE|GLISTENER_EVENTBUSY);
 		
 	// Loop through the table looking for attachments to this source
 	for(psl = lastlr ? (lastlr+1) : Assignments; psl < Assignments+GEVENT_MAX_SOURCE_LISTENERS; psl++) {
 		if (gsh == psl->pSource) {
-			gfxSemWait(&psl->pListener->eventlock, TIME_INFINITE);		// Obtain a lock on the listener event buffer
 			gfxMutexExit(&geventMutex);
 			return psl;
 		}
@@ -163,21 +178,38 @@ GSourceListener *geventGetSourceListener(GSourceHandle gsh, GSourceListener *las
 }
 
 GEvent *geventGetEventBuffer(GSourceListener *psl) {
-	// We already know we have the event lock
-	return &psl->pListener->callback || gfxSemCounter(&psl->pListener->waitqueue) < 0 ? &psl->pListener->event : 0;
+	gfxMutexEnter(&geventMutex);
+	if ((psl->pListener->flags & GLISTENER_EVENTBUSY)) {
+		// Oops - event buffer is still in use
+		gfxMutexExit(&geventMutex);
+		return 0;
+	}
+
+	// Allocate the event buffer
+	psl->pListener->flags |= (GLISTENER_WITHSOURCE|GLISTENER_EVENTBUSY);
+	gfxMutexExit(&geventMutex);
+	return &psl->pListener->event;
 }
 
 void geventSendEvent(GSourceListener *psl) {
 	gfxMutexEnter(&geventMutex);
-	if (psl->pListener->callback) {				// This test needs to be taken inside the mutex
+	if (psl->pListener->callback) {
+
+		// Mark it back as free and as sent. This is early to be marking as free but it protects
+		//	if the callback alters the listener in any way
+		psl->pListener->flags &= ~(GLISTENER_WITHSOURCE|GLISTENER_EVENTBUSY);
 		gfxMutexExit(&geventMutex);
-		// We already know we have the event lock
+
+		// Do the callback
 		psl->pListener->callback(psl->pListener->param, &psl->pListener->event);
 
 	} else {
 		// Wake up the listener
-		if (gfxSemCounter(&psl->pListener->waitqueue) <= 0)
+		if ((psl->pListener->flags & GLISTENER_WAITING)) {
+			psl->pListener->flags &= ~(GLISTENER_WITHSOURCE|GLISTENER_WAITING);
 			gfxSemSignal(&psl->pListener->waitqueue);
+			// The listener thread will free the event buffer when ready
+		}
 		gfxMutexExit(&geventMutex);
 	}
 }
diff --git a/src/gevent/sys_defs.h b/src/gevent/sys_defs.h
index c50dc5ae..9f1f4dde 100644
--- a/src/gevent/sys_defs.h
+++ b/src/gevent/sys_defs.h
@@ -56,7 +56,7 @@ typedef void (*GEventCallbackFn)(void *param, GEvent *pe);
 // The Listener Object
 typedef struct GListener {
 	gfxSem				waitqueue;			// Private: Semaphore for the listener to wait on.
-	gfxSem				eventlock;			// Private: Protect against more than one sources trying to use this event lock at the same time
+	uint16_t			flags;				// Private: Flags for operation
 	GEventCallbackFn	callback;			// Private: Call back Function
 	void				*param;				// Private: Parameter for the callback function.
 	GEvent				event;				// Public:  The event object into which the event information is stored.
@@ -149,9 +149,11 @@ void geventDetachSource(GListener *pl, GSourceHandle gsh);
  * 			timeout specifies the time to wait in system ticks.
  *			TIME_INFINITE means no timeout - wait forever for an event.
  *			TIME_IMMEDIATE means return immediately
- * @note	The GEvent buffer is staticly allocated within the GListener so the event does not
- *			need to be dynamicly freed however it will get overwritten by the next call to
- *			this routine.
+ * @note	The returned GEvent is released when this routine is called again
+ * 			or when optionally @p geventEventComplete() is called. Calling @p geventEventComplete()
+ * 			allows the GEvent object to be reused earlier which can reduce missed events. The GEvent
+ * 			object MUST NOT be used after this function is called (and is blocked waiting for the next
+ * 			event) or after geventEventComplete() is called.
  *
  * @param[in] pl		The listener
  * @param[in] timeout	The timeout
@@ -160,6 +162,17 @@ void geventDetachSource(GListener *pl, GSourceHandle gsh);
  */
 GEvent *geventEventWait(GListener *pl, delaytime_t timeout);
 
+/**
+ * @brief	Release the GEvent buffer associated with a listener.
+ * @details	The GEvent returned by @p geventEventWait() is released.
+ * @note	The GEvent pointer returned by @p geventEventWait() is released when @p geventEventWait()
+ * 			is called again or when this function is called. The GEvent
+ * 			object MUST NOT be used after this function is called.
+ *
+ * @param[in] pl		The listener
+ */
+void geventEventComplete(GListener *pl);
+
 /* @brief	Register a callback for an event on a listener from an assigned source.
  * @details	The type of the event should be checked (pevent->type) and then pevent should be typecast to the
  *			actual event type if it needs to be processed.