Browse Source

Fixes to GEVENT

inmarket 4 years ago
parent
commit
e0eacb846c
3 changed files with 42 additions and 52 deletions
  1. 1 0
      docs/releases.txt
  2. 37 49
      src/gevent/gevent.c
  3. 4 3
      src/gevent/gevent.h

+ 1 - 0
docs/releases.txt

@@ -23,6 +23,7 @@ FEATURE:	Added high-level functions to modify image color palettes
23 23
 FIX:		Improving gdispDrawThickLine()
24 24
 FEATURE:	Added gdispAddFont() for adding a dynamic font to the permanent font list
25 25
 FEATURE:	Added gmiscHittestPoly() for checking whether a point is inside of a polygon
26
+FIX:		Fixed strange multi-thread issues in GEVENT
26 27
 
27 28
 
28 29
 *** Release 2.6 ***

+ 37 - 49
src/gevent/gevent.c

@@ -16,10 +16,8 @@
16 16
 #endif
17 17
 
18 18
 /* Flags in the listener structure */
19
-#define GLISTENER_EVENTBUSY			0x0001			// The event buffer is busy
20
-#define GLISTENER_WAITING			0x0002			// The listener is waiting for a signal
21
-#define GLISTENER_WITHSOURCE		0x0004			// The listener is being looked at by a source for a possible event
22
-#define GLISTENER_PENDING			0x0008			// There is an event waiting ready to go without a current listener
19
+#define GLISTENER_WITHLISTENER		0x0001			// The listener is current using the buffer
20
+#define GLISTENER_WITHSOURCE		0x0002			// The source is currently using the buffer
23 21
 
24 22
 /* This mutex protects access to our tables */
25 23
 static gfxMutex	geventMutex;
@@ -30,11 +28,10 @@ static GSourceListener		Assignments[GEVENT_MAX_SOURCE_LISTENERS];
30 28
 /* Send an exit event if possible. */
31 29
 /* We already have the geventMutex */
32 30
 static void doExitEvent(GListener *pl) {
33
-	// Don't do the exit if someone else currently has the event lock
34
-	if ((pl->flags & (GLISTENER_WAITING|GLISTENER_EVENTBUSY)) == GLISTENER_WAITING) {
35
-		pl->flags |= GLISTENER_EVENTBUSY;							// Event buffer is in use
31
+	// Don't do the exit if someone else currently is using the buffer
32
+	if (!(pl->flags & GLISTENER_WITHLISTENER)) {
36 33
 		pl->event.type = GEVENT_EXIT;								// Set up the EXIT event
37
-		pl->flags &= ~GLISTENER_WAITING;							// Wake up the listener (with data)
34
+		pl->flags = GLISTENER_WITHLISTENER;							// Event buffer is now in use by the listener
38 35
 		gfxSemSignal(&pl->waitqueue);
39 36
 	}
40 37
 }
@@ -119,38 +116,36 @@ void geventDetachSource(GListener *pl, GSourceHandle gsh) {
119 116
 }
120 117
 
121 118
 GEvent *geventEventWait(GListener *pl, delaytime_t timeout) {
122
-	gfxMutexEnter(&geventMutex);
123
-	// Don't allow waiting if we are on callbacks or if there is already a thread waiting
124
-	if (pl->callback || (pl->flags & GLISTENER_WAITING)) {
125
-		gfxMutexExit(&geventMutex);
119
+	/* NOTE:
120
+		We no longer try to protect against two threads trying to listen on the
121
+		one listener. This was never allowed, it makes little sense to try to do so,
122
+		and the testing caused strange multi-thread windows of opportunity.
123
+		
124
+		In practice it is probably safer than it used to be - the only potential
125
+		issue is that the buffer may be prematurely marked as not in use by the listener.
126
+		If the calling code can guarantee that the event buffer is free when either thread
127
+		calls the event wait - it is now safe for them to do so.
128
+		ie. it is the implicit geventEventComplete() that is the only thing that now raises
129
+		possible multi-thread issues.
130
+	*/
131
+
132
+	// Don't allow waiting if we are on callbacks
133
+	if (pl->callback)
126 134
 		return 0;
127
-	}
128
-
129
-	// Check to see if there is a pending event ready for us
130
-	if ((pl->flags & GLISTENER_PENDING)) {
131
-		pl->flags &= ~GLISTENER_PENDING;				// We have now got this
132
-		pl->flags |= GLISTENER_EVENTBUSY;				// Event buffer is definitely busy
133
-		gfxMutexExit(&geventMutex);
134
-		return &pl->event;
135
-	}
135
+		
136
+	// Event buffer is not in use by the listener - this is an implicit geventEventComplete() call
137
+	pl->flags &= ~GLISTENER_WITHLISTENER;
136 138
 
137
-	// No - wait for one.
138
-	pl->flags &= ~GLISTENER_EVENTBUSY;				// Event buffer is definitely not busy
139
-	pl->flags |= GLISTENER_WAITING;					// We will now be waiting on the thread
140
-	gfxMutexExit(&geventMutex);
141
-	if (gfxSemWait(&pl->waitqueue, timeout))
142
-		return &pl->event;
139
+	// Wait for an event
140
+	if (!gfxSemWait(&pl->waitqueue, timeout))
141
+		return 0;				// Timeout
143 142
 
144
-	// Timeout - clear the waiting flag.
145
-	// We know this is safe as any other thread will still think there is someone waiting.
146
-	gfxMutexEnter(&geventMutex);
147
-	pl->flags &= ~GLISTENER_WAITING;
148
-	gfxMutexExit(&geventMutex);
149
-	return 0;
143
+	return &pl->event;
150 144
 }
151 145
 
152 146
 void geventEventComplete(GListener *pl) {
153
-	pl->flags &= ~GLISTENER_EVENTBUSY;
147
+	// The listener is done with the buffer
148
+	pl->flags &= ~GLISTENER_WITHLISTENER;
154 149
 }
155 150
 
156 151
 void geventRegisterCallback(GListener *pl, GEventCallbackFn fn, void *param) {
@@ -160,7 +155,7 @@ void geventRegisterCallback(GListener *pl, GEventCallbackFn fn, void *param) {
160 155
 		pl->param = param;						// Set the param
161 156
 		pl->callback = fn;						// Set the callback function
162 157
 		if (fn)
163
-			pl->flags &= ~GLISTENER_EVENTBUSY;	// The event buffer is immediately available
158
+			pl->flags &= ~GLISTENER_WITHLISTENER;	// The event buffer is immediately available
164 159
 		gfxMutexExit(&geventMutex);
165 160
 	}
166 161
 }
@@ -176,7 +171,7 @@ GSourceListener *geventGetSourceListener(GSourceHandle gsh, GSourceListener *las
176 171
 
177 172
 	// Unlock the last listener event buffer if it wasn't used.
178 173
 	if (lastlr && lastlr->pListener && (lastlr->pListener->flags & GLISTENER_WITHSOURCE))
179
-		lastlr->pListener->flags &= ~(GLISTENER_WITHSOURCE|GLISTENER_EVENTBUSY);
174
+		lastlr->pListener->flags &= ~GLISTENER_WITHSOURCE;
180 175
 		
181 176
 	// Loop through the table looking for attachments to this source
182 177
 	for(psl = lastlr ? (lastlr+1) : Assignments; psl < Assignments+GEVENT_MAX_SOURCE_LISTENERS; psl++) {
@@ -191,14 +186,13 @@ GSourceListener *geventGetSourceListener(GSourceHandle gsh, GSourceListener *las
191 186
 
192 187
 GEvent *geventGetEventBuffer(GSourceListener *psl) {
193 188
 	gfxMutexEnter(&geventMutex);
194
-	if ((psl->pListener->flags & GLISTENER_EVENTBUSY)) {
195
-		// Oops - event buffer is still in use
189
+	if ((psl->pListener->flags & (GLISTENER_WITHLISTENER|GLISTENER_WITHSOURCE))) {
196 190
 		gfxMutexExit(&geventMutex);
197 191
 		return 0;
198 192
 	}
199 193
 
200
-	// Allocate the event buffer
201
-	psl->pListener->flags |= (GLISTENER_WITHSOURCE|GLISTENER_EVENTBUSY);
194
+	// Allocate the event buffer to the source
195
+	psl->pListener->flags |= GLISTENER_WITHSOURCE;
202 196
 	gfxMutexExit(&geventMutex);
203 197
 	return &psl->pListener->event;
204 198
 }
@@ -209,7 +203,7 @@ void geventSendEvent(GSourceListener *psl) {
209 203
 
210 204
 		// Mark it back as free and as sent. This is early to be marking as free but it protects
211 205
 		//	if the callback alters the listener in any way
212
-		psl->pListener->flags &= ~(GLISTENER_WITHSOURCE|GLISTENER_EVENTBUSY|GLISTENER_PENDING);
206
+		psl->pListener->flags = 0;
213 207
 		gfxMutexExit(&geventMutex);
214 208
 
215 209
 		// Do the callback
@@ -217,14 +211,8 @@ void geventSendEvent(GSourceListener *psl) {
217 211
 
218 212
 	} else {
219 213
 		// Wake up the listener
220
-		psl->pListener->flags &= ~GLISTENER_WITHSOURCE;
221
-		if ((psl->pListener->flags & GLISTENER_WAITING)) {
222
-			psl->pListener->flags &= ~(GLISTENER_WAITING|GLISTENER_PENDING);
223
-			gfxSemSignal(&psl->pListener->waitqueue);
224
-		} else
225
-			psl->pListener->flags |= GLISTENER_PENDING;
226
-
227
-		// The listener thread will free the event buffer when ready
214
+		psl->pListener->flags = GLISTENER_WITHLISTENER;
215
+		gfxSemSignal(&psl->pListener->waitqueue);
228 216
 		gfxMutexExit(&geventMutex);
229 217
 	}
230 218
 }

+ 4 - 3
src/gevent/gevent.h

@@ -146,7 +146,6 @@ void geventDetachSource(GListener *pl, GSourceHandle gsh);
146 146
  * @brief	Wait for an event on a listener from an assigned source.
147 147
  * @details	The type of the event should be checked (pevent->type) and then pevent should
148 148
  *			be typecast to the actual event type if it needs to be processed.
149
- * 			timeout specifies the time to wait in system ticks.
150 149
  *			TIME_INFINITE means no timeout - wait forever for an event.
151 150
  *			TIME_IMMEDIATE means return immediately
152 151
  * @note	The returned GEvent is released when this routine is called again
@@ -154,9 +153,11 @@ void geventDetachSource(GListener *pl, GSourceHandle gsh);
154 153
  * 			allows the GEvent object to be reused earlier which can reduce missed events. The GEvent
155 154
  * 			object MUST NOT be used after this function is called (and is blocked waiting for the next
156 155
  * 			event) or after geventEventComplete() is called.
156
+ *			The one listener object should not be waited on using more than one thread simultanously
157
+ *			because of the implicit geventEventComplete() that occurs when this function is called.
157 158
  *
158 159
  * @param[in] pl		The listener
159
- * @param[in] timeout	The timeout
160
+ * @param[in] timeout	The timeout in milliseconds
160 161
  *
161 162
  * @return	NULL on timeout
162 163
  */
@@ -216,7 +217,7 @@ GSourceListener *geventGetSourceListener(GSourceHandle gsh, GSourceListener *las
216 217
  *
217 218
  * @param[in] psl	The source listener
218 219
  *
219
- * @return	NULL if the listener is not currently listening.
220
+ * @return	NULL if the event buffer for this listener is currently in use.
220 221
  */
221 222
 GEvent *geventGetEventBuffer(GSourceListener *psl);
222 223