libwinpr-synch: improve timer queue implementation

Gilsondc · Jan 27, 2014 · 159f539 · 159f539
1 parent 1f394eb
commit 159f539
Show file tree

Hide file tree

Showing 3 changed files with 133 additions and 13 deletions.
diff --git a/winpr/libwinpr/synch/synch.h b/winpr/libwinpr/synch/synch.h
@@ -120,6 +120,7 @@ struct winpr_timer_queue
 	pthread_mutex_t cond_mutex;
 	struct sched_param param;
 
+	BOOL bCancelled;
 	WINPR_TIMER_QUEUE_TIMER* head;
 };
 typedef struct winpr_timer_queue WINPR_TIMER_QUEUE;

diff --git a/winpr/libwinpr/synch/test/TestSynchTimerQueue.c b/winpr/libwinpr/synch/test/TestSynchTimerQueue.c
@@ -35,12 +35,12 @@ VOID CALLBACK TimerRoutine(PVOID lpParam, BOOLEAN TimerOrWaitFired)
 	TimerTime = CurrentTime - apcData->StartTime;
 	expectedTime = apcData->DueTime + (apcData->Period * apcData->FireCount);
 
-	printf("TimerRoutine: TimerId: %d ActualTime: %d ExpectedTime: %d Discrepancy: %d\n",
-			apcData->TimerId, TimerTime, expectedTime, TimerTime - expectedTime);
-
 	apcData->FireCount++;
 	g_Count++;
 
+	printf("TimerRoutine: TimerId: %d FireCount: %d ActualTime: %d ExpectedTime: %d Discrepancy: %d\n",
+			apcData->TimerId, apcData->FireCount, TimerTime, expectedTime, TimerTime - expectedTime);
+
 	if (g_Count >= (TIMER_COUNT * FIRE_COUNT))
 	{
 		SetEvent(g_Event);
@@ -50,8 +50,8 @@ VOID CALLBACK TimerRoutine(PVOID lpParam, BOOLEAN TimerOrWaitFired)
 int TestSynchTimerQueue(int argc, char* argv[])
 {
 	int index;
-	HANDLE hTimer = NULL;
-	HANDLE hTimerQueue = NULL;
+	HANDLE hTimerQueue;
+	HANDLE hTimers[TIMER_COUNT];
 	APC_DATA apcData[TIMER_COUNT];
 
 	g_Event = CreateEvent(NULL, TRUE, FALSE, NULL);
@@ -72,7 +72,7 @@ int TestSynchTimerQueue(int argc, char* argv[])
 		apcData[index].Period = 1000;
 		apcData[index].FireCount = 0;
 
-		if (!CreateTimerQueueTimer(&hTimer, hTimerQueue, (WAITORTIMERCALLBACK) TimerRoutine,
+		if (!CreateTimerQueueTimer(&hTimers[index], hTimerQueue, (WAITORTIMERCALLBACK) TimerRoutine,
 				&apcData[index], apcData[index].DueTime, apcData[index].Period, 0))
 		{
 			printf("CreateTimerQueueTimer failed (%d)\n", GetLastError());
@@ -86,10 +86,13 @@ int TestSynchTimerQueue(int argc, char* argv[])
 		return -1;
 	}
 
-	if (!DeleteTimerQueueTimer(hTimerQueue, hTimer, NULL))
+	for (index = 0; index < TIMER_COUNT; index++)
 	{
-		printf("DeleteTimerQueueTimer failed (%d)\n", GetLastError());
-		return -1;
+		if (!DeleteTimerQueueTimer(hTimerQueue, hTimers[index], NULL))
+		{
+			printf("DeleteTimerQueueTimer failed (%d)\n", GetLastError());
+			return -1;
+		}
 	}
 
 	if (!DeleteTimerQueue(hTimerQueue))

diff --git a/winpr/libwinpr/synch/timer.c b/winpr/libwinpr/synch/timer.c
@@ -22,11 +22,13 @@
 #endif
 
 #include <winpr/crt.h>
+#include <winpr/file.h>
 #include <winpr/sysinfo.h>
 
 #include <winpr/synch.h>
 
 #ifndef _WIN32
+#include <errno.h>
 #include <sys/time.h>
 #include <signal.h>
 #endif
@@ -384,6 +386,33 @@ void InsertTimerQueueTimer(WINPR_TIMER_QUEUE_TIMER** pHead, WINPR_TIMER_QUEUE_TI
 	node->next = timer;
 }
 
+void RemoveTimerQueueTimer(WINPR_TIMER_QUEUE_TIMER** pHead, WINPR_TIMER_QUEUE_TIMER* timer)
+{
+	WINPR_TIMER_QUEUE_TIMER* node;
+	WINPR_TIMER_QUEUE_TIMER* prevNode;
+
+	if (timer == *pHead)
+	{
+		*pHead = timer->next;
+		return;
+	}
+
+	node = *pHead;
+	prevNode = NULL;
+
+	while (node)
+	{
+		if (node == timer)
+			break;
+
+		prevNode = node;
+		node = node->next;
+	}
+
+	prevNode->next = timer->next;
+	timer->next = NULL;
+}
+
 int FireExpiredTimerQueueTimers(WINPR_TIMER_QUEUE* timerQueue)
 {
 	struct timespec CurrentTime;
@@ -403,13 +432,13 @@ int FireExpiredTimerQueueTimers(WINPR_TIMER_QUEUE* timerQueue)
 			node->Callback(node->Parameter, TRUE);
 			node->FireCount++;
 
+			timerQueue->head = node->next;
+			node->next = NULL;
+
 			if (node->Period)
 			{
 				timespec_add_ms(&(node->ExpirationTime), node->Period);
 
-				timerQueue->head = node->next;
-				node->next = NULL;
-
 				InsertTimerQueueTimer(&(timerQueue->head), node);
 				node = timerQueue->head;
 			}
@@ -436,7 +465,7 @@ static void* TimerQueueThread(void* arg)
 		if (!timerQueue->head)
 		{
 			timespec_gettimeofday(&timeout);
-			timespec_add_ms(&timeout, 20);
+			timespec_add_ms(&timeout, 100);
 		}
 		else
 		{
@@ -448,6 +477,9 @@ static void* TimerQueueThread(void* arg)
 		FireExpiredTimerQueueTimers(timerQueue);
 
 		pthread_mutex_unlock(&(timerQueue->cond_mutex));
+
+		if (timerQueue->bCancelled)
+			break;
 	}
 
 	return NULL;
@@ -481,6 +513,9 @@ HANDLE CreateTimerQueue(void)
 		WINPR_HANDLE_SET_TYPE(timerQueue, HANDLE_TYPE_TIMER_QUEUE);
 		handle = (HANDLE) timerQueue;
 
+		timerQueue->head = NULL;
+		timerQueue->bCancelled = FALSE;
+
 		StartTimerQueueThread(timerQueue);
 	}
 
@@ -489,13 +524,51 @@ HANDLE CreateTimerQueue(void)
 
 BOOL DeleteTimerQueueEx(HANDLE TimerQueue, HANDLE CompletionEvent)
 {
+	void* rvalue;
 	WINPR_TIMER_QUEUE* timerQueue;
+	WINPR_TIMER_QUEUE_TIMER* node;
+	WINPR_TIMER_QUEUE_TIMER* nextNode;
 
 	if (!TimerQueue)
 		return FALSE;
 
 	timerQueue = (WINPR_TIMER_QUEUE*) TimerQueue;
 
+	/* Cancel and delete timer queue timers */
+
+	pthread_mutex_lock(&(timerQueue->cond_mutex));
+
+	timerQueue->bCancelled = TRUE;
+
+	pthread_cond_signal(&(timerQueue->cond));
+	pthread_mutex_unlock(&(timerQueue->cond_mutex));
+
+	pthread_join(timerQueue->thread, &rvalue);
+
+	if (CompletionEvent == INVALID_HANDLE_VALUE)
+	{
+		/* Wait for all callback functions to complete before returning */
+	}
+	else
+	{
+		/* Cancel all timers and return immediately */
+
+		node = timerQueue->head;
+
+		while (node)
+		{
+			nextNode = node->next;
+
+			free(node);
+
+			node = nextNode;
+		}
+
+		timerQueue->head = NULL;
+	}
+
+	/* Delete timer queue */
+
 	pthread_cond_destroy(&(timerQueue->cond));
 	pthread_mutex_destroy(&(timerQueue->cond_mutex));
 
@@ -505,6 +578,9 @@ BOOL DeleteTimerQueueEx(HANDLE TimerQueue, HANDLE CompletionEvent)
 
 	free(timerQueue);
 
+	if (CompletionEvent && (CompletionEvent != INVALID_HANDLE_VALUE))
+		SetEvent(CompletionEvent);
+
 	return TRUE;
 }
 
@@ -542,8 +618,13 @@ BOOL CreateTimerQueueTimer(PHANDLE phNewTimer, HANDLE TimerQueue,
 
 	timer->FireCount = 0;
 
+	pthread_mutex_lock(&(timerQueue->cond_mutex));
+
 	InsertTimerQueueTimer(&(timerQueue->head), timer);
 
+	pthread_cond_signal(&(timerQueue->cond));
+	pthread_mutex_unlock(&(timerQueue->cond_mutex));
+
 	return TRUE;
 }
 
@@ -558,6 +639,22 @@ BOOL ChangeTimerQueueTimer(HANDLE TimerQueue, HANDLE Timer, ULONG DueTime, ULONG
 	timerQueue = (WINPR_TIMER_QUEUE*) TimerQueue;
 	timer = (WINPR_TIMER_QUEUE_TIMER*) Timer;
 
+	pthread_mutex_lock(&(timerQueue->cond_mutex));
+
+	RemoveTimerQueueTimer(&(timerQueue->head), timer);
+
+	timer->DueTime = DueTime;
+	timer->Period = Period;
+
+	timespec_gettimeofday(&(timer->StartTime));
+	timespec_add_ms(&(timer->StartTime), DueTime);
+	timespec_copy(&(timer->ExpirationTime), &(timer->StartTime));
+
+	InsertTimerQueueTimer(&(timerQueue->head), timer);
+
+	pthread_cond_signal(&(timerQueue->cond));
+	pthread_mutex_unlock(&(timerQueue->cond_mutex));
+
 	return TRUE;
 }
 
@@ -572,8 +669,27 @@ BOOL DeleteTimerQueueTimer(HANDLE TimerQueue, HANDLE Timer, HANDLE CompletionEve
 	timerQueue = (WINPR_TIMER_QUEUE*) TimerQueue;
 	timer = (WINPR_TIMER_QUEUE_TIMER*) Timer;
 
+	pthread_mutex_lock(&(timerQueue->cond_mutex));
+
+	if (CompletionEvent == INVALID_HANDLE_VALUE)
+	{
+		/* Wait for all callback functions to complete before returning */
+	}
+	else
+	{
+		/* Cancel timer and return immediately */
+
+		RemoveTimerQueueTimer(&(timerQueue->head), timer);
+	}
+
+	pthread_cond_signal(&(timerQueue->cond));
+	pthread_mutex_unlock(&(timerQueue->cond_mutex));
+
 	free(timer);
 
+	if (CompletionEvent && (CompletionEvent != INVALID_HANDLE_VALUE))
+		SetEvent(CompletionEvent);
+
 	return TRUE;
 }