cairo-dock-team team mailing list archive

[Matthieu Baerts <matttbe@xxxxxxxxx>]

Matthieu Baerts has proposed merging lp:~matttbe/cairo-dock-core/task-one-thread-async-new-struct into lp:cairo-dock-core.

Requested reviews:
  Fabounet (fabounet03)

For more details, see:
https://code.launchpad.net/~matttbe/cairo-dock-core/task-one-thread-async-new-struct/+merge/120581

A better implementation of the task.
Now thread's datas are separated in order to better prevent crash if the task is discarded and allow a task to be stopped before the end of 'get_data' and re-launch a new thread.
-- 
https://code.launchpad.net/~matttbe/cairo-dock-core/task-one-thread-async-new-struct/+merge/120581
Your team Cairo-Dock Team is subscribed to branch lp:cairo-dock-core.

=== modified file 'src/gldit/cairo-dock-task.c'
--- src/gldit/cairo-dock-task.c	2012-08-02 17:43:50 +0000
+++ src/gldit/cairo-dock-task.c	2012-08-21 14:25:23 +0000
@@ -24,99 +24,183 @@
 #include "cairo-dock-log.h"
 #include "cairo-dock-task.h"
 
-
-#define cairo_dock_schedule_next_iteration(pTask) do {\
-	if (pTask->iSidTimer == 0 && pTask->iPeriod)\
-		pTask->iSidTimer = g_timeout_add_seconds (pTask->iPeriod, (GSourceFunc) _cairo_dock_timer, pTask); } while (0)
-
-#define cairo_dock_cancel_next_iteration(pTask) do {\
-	if (pTask->iSidTimer != 0) {\
-		g_source_remove (pTask->iSidTimer);\
-		pTask->iSidTimer = 0; } } while (0)
-
-#define cairo_dock_perform_task_update(pTask) do {\
-	gboolean bContinue = pTask->update (pTask->pSharedMemory);\
-	if (! bContinue) {\
-		cairo_dock_cancel_next_iteration (pTask); }\
-	else {\
-		pTask->iFrequencyState = CAIRO_DOCK_FREQUENCY_NORMAL;\
-		cairo_dock_schedule_next_iteration (pTask); } } while (0)
-
-#define cairo_dock_set_elapsed_time(pTask) do {\
-	pTask->fElapsedTime = g_timer_elapsed (pTask->pClock, NULL);\
-	g_timer_start (pTask->pClock); } while (0)
-
-#define _free_task(pTask) do {\
-	if (pTask->free_data)\
-		pTask->free_data (pTask->pSharedMemory);\
-	g_timer_destroy (pTask->pClock);\
-	g_free (pTask); } while (0)
+#if (GLIB_MAJOR_VERSION == 2 && GLIB_MINOR_VERSION < 32)
+#define CD_GMUTEX pThread->pMutex
+#define CD_TASK_GMUTEX pTask->CD_GMUTEX
+#else
+#define CD_GMUTEX &pThread->aMutex
+#define CD_TASK_GMUTEX &pTask->pThread->aMutex
+#endif
+
 
 static gboolean _cairo_dock_timer (CairoDockTask *pTask)
 {
 	cairo_dock_launch_task (pTask);
 	return TRUE;
 }
+
+static void cairo_dock_schedule_next_iteration (CairoDockTask *pTask)
+{
+	if (pTask->iSidTimer == 0 && pTask->iPeriod)
+		pTask->iSidTimer = g_timeout_add_seconds (pTask->iPeriod, (GSourceFunc) _cairo_dock_timer, pTask);
+}
+
+static void cairo_dock_cancel_next_iteration (CairoDockTask *pTask)
+{
+	if (! pTask || pTask->bDiscard) // already did in cairo_dock_discard_task (_free_disc. -> free -> stop -> pause -> _free)
+		return;
+	if (pTask->iSidTimer != 0)
+	{
+		g_source_remove (pTask->iSidTimer);
+		pTask->iSidTimer = 0;
+	}
+	if (pTask->get_data != NULL)
+	{
+		g_print ("Task: Unlock thread if it's running (%p)\n", pTask);
+		// we notify the thread that it can stop
+		g_atomic_int_set (&pTask->pThread->iUpdateIsEnded, 0); // just to be sure
+		g_atomic_int_set (&pTask->pThread->iThreadCanRun, 0);
+		// Maybe the thread is waiting
+		if (pTask->pThread && g_atomic_int_get (&pTask->pThread->iThreadIsRunning) == 0)
+		{
+			g_mutex_unlock (CD_TASK_GMUTEX); // unlock to stop the thread
+			g_print ("Task: Unlocked (%p)\n", pTask);
+		}
+		pTask->pThread = NULL; // now, if we want to relaunch the task, we'll have to relaunch a new thread
+		/// g_mutex_unlock (CD_GMUTEX); // locked at the beginning of the thread or in the thread
+		/// g_mutex_trylock (CD_GMUTEX); // if was 
+		/// g_mutex_unlock (CD_GMUTEX);
+	}
+}
+
+static void cairo_dock_perform_task_update (CairoDockTask *pTask)
+{
+	gboolean bContinue = pTask->update (pTask->pSharedMemory);
+	if (! bContinue)
+		cairo_dock_cancel_next_iteration (pTask);
+	else
+	{
+		pTask->iFrequencyState = CAIRO_DOCK_FREQUENCY_NORMAL;
+		cairo_dock_schedule_next_iteration (pTask);
+	}
+}
+
+static void cairo_dock_set_elapsed_time (CairoDockTask *pTask)
+{
+	pTask->fElapsedTime = g_timer_elapsed (pTask->pClock, NULL);
+	g_timer_start (pTask->pClock);
+}
+
+static gboolean _cairo_dock_check_for_update (CairoDockTask *pTask)
+{
+	if (! pTask || pTask->bDiscard || ! pTask->pThread)  // task has been discarded
+		return FALSE;
+	if (g_atomic_int_get (&pTask->pThread->iThreadIsRunning) == 0)  // data have been produced by the thread
+	{
+		g_print ("Task: Perform task update (%p)\n", pTask);
+		if (pTask->bDiscard)  // task has been discarded
+			return FALSE;
+
+		pTask->iSidTimerUpdate = 0; // timer for the update
+		// We can perform task update and continue/stop the task's timer.
+		cairo_dock_perform_task_update (pTask);
+
+		return FALSE;
+	}
+	return TRUE; // continue to check if it's possible to perform task update
+}
+
 static gpointer _cairo_dock_threaded_calculation (CairoDockTask *pTask)
 {
-	//\_______________________ On obtient nos donnees.
-	cairo_dock_set_elapsed_time (pTask);
-	pTask->get_data (pTask->pSharedMemory);
-	
-	//\_______________________ On indique qu'on a fini.
-	g_atomic_int_set (&pTask->iThreadIsRunning, 0);
+	CairoDockTaskThread *pThread = pTask->pThread; // needed for the thread
+	g_print ("Task: Start a new thread (%p - %p)\n", pTask, pThread);
+	if (! pThread || ! g_mutex_trylock (CD_GMUTEX)
+		|| ! g_atomic_int_compare_and_exchange (&pThread->iThreadIsRunning, 0, 1))
+		return NULL; // was locked and is running: should not happen...
+	while (g_atomic_int_get (&pThread->iThreadCanRun) == 1)
+	{
+		if (pTask && pTask->bDiscard) // discarded just after its launch...
+			break;
+
+		g_print ("Task: thread: Get data (%p - %p)\n", pTask, pThread);
+		//\_______________________ Get data
+		cairo_dock_set_elapsed_time (pTask);
+		pTask->get_data (pTask->pSharedMemory);
+		g_atomic_int_set (&pThread->iUpdateIsEnded, 1);
+		g_print ("Task: thread: data ok (%p - %p)\n", pTask, pThread);
+		
+		if (g_atomic_int_get (&pThread->iThreadCanRun) == 0 || (pTask && pTask->bDiscard))
+			break; // if the task has been cancelled, we should not wait... we should stop !
+
+		// we launch the update in the main loop
+		if (pTask->iSidTimerUpdate == 0)
+			pTask->iSidTimerUpdate = g_idle_add ((GSourceFunc) _cairo_dock_check_for_update, pTask); 
+		g_print ("Task: thread: g idle (%p, %p) - %d\n", pTask, pThread, pTask->iSidTimerUpdate);
+
+		g_atomic_int_set (&pThread->iThreadIsRunning, 0);
+
+		//\_______________________ We lock to wait for the next update
+		if (g_atomic_int_get (&pThread->iUpdateIsEnded) == 1) // TODO: is it possible? it takes a few time to launch the update in the main loop + wait for the end + the timer... Maybe... if we stop the task.
+			g_mutex_lock (CD_GMUTEX);
+	}
+	g_atomic_int_set (&pThread->iThreadIsRunning, -1); // the task can be freed
+
+	g_mutex_unlock (CD_GMUTEX); // was locked at the beginning of the thread
+
+	//\______________________ Free
+	#if (GLIB_MAJOR_VERSION == 2 && GLIB_MINOR_VERSION < 32)
+	g_mutex_free (CD_GMUTEX);
+	#else
+	g_mutex_clear (CD_GMUTEX);
+	#endif
+	g_free (pThread);
+	g_print ("Task: Stop the thread (%p - %p)\n", pTask, pThread);
 	return NULL;
 }
-static gboolean _cairo_dock_check_for_update (CairoDockTask *pTask)
-{
-	int iThreadIsRunning = g_atomic_int_get (&pTask->iThreadIsRunning);
-	if (! iThreadIsRunning)  // le thread a fini.
-	{
-		if (pTask->bDiscard)  // la tache s'est faite abandonnee.
-		{
-			//g_print ("free discared task...\n");
-			_free_task (pTask);
-			//g_print ("done.\n");
-			return FALSE;
-		}
-		
-		// On met a jour avec ces nouvelles donnees et on lance/arrete le timer.
-		pTask->iSidTimerUpdate = 0;
-		cairo_dock_perform_task_update (pTask);
-		
-		return FALSE;
-	}
-	return TRUE;
-}
+
 void cairo_dock_launch_task (CairoDockTask *pTask)
 {
 	g_return_if_fail (pTask != NULL);
-	if (pTask->get_data == NULL)  // pas de thread, tout est dans la fonction d'update.
+	if (pTask->get_data == NULL)  // no threads, only update
 	{
 		cairo_dock_set_elapsed_time (pTask);
 		cairo_dock_perform_task_update (pTask);
 	}
 	else
 	{
-		if (g_atomic_int_compare_and_exchange (&pTask->iThreadIsRunning, 0, 1))  // il etait egal a 0, on lui met 1 et on lance le thread.
+		if (pTask->pThread == NULL) //g_atomic_int_compare_and_exchange (&pTask->iThreadCanRun, 0, 1) // if was 0, now 1 => we can launch a new thread
 		{
-			GError *erreur = NULL;
+			GError *error = NULL;
+
+			pTask->pThread = g_new0 (CairoDockTaskThread, 1);
+			pTask->pThread->iThreadCanRun = 1;
+
 			#if (GLIB_MAJOR_VERSION == 2 && GLIB_MINOR_VERSION < 32)
-			GThread* pThread = g_thread_create ((GThreadFunc) _cairo_dock_threaded_calculation, pTask, FALSE, &erreur);
+			CD_TASK_GMUTEX = g_mutex_new ();
+			g_thread_create ((GThreadFunc) _cairo_dock_threaded_calculation, pTask, FALSE, &error);
 			#else
-			GThread* pThread = g_thread_try_new ("Cairo-Dock Task", (GThreadFunc) _cairo_dock_threaded_calculation, pTask, &erreur);
+			g_mutex_init (CD_TASK_GMUTEX);
+			GThread* pThread = g_thread_try_new ("Task", (GThreadFunc) _cairo_dock_threaded_calculation, pTask, &error);
 			g_thread_unref (pThread);
 			#endif
-			if (erreur != NULL)  // on n'a pas pu lancer le thread.
+
+			if (error != NULL)
 			{
-				cd_warning (erreur->message);
-				g_error_free (erreur);
-				g_atomic_int_set (&pTask->iThreadIsRunning, 0);
+				cd_warning (error->message);
+				g_error_free (error);
+				pTask->pThread->iThreadCanRun = 0;
 			}
 		}
+		else if (pTask->iSidTimerUpdate == 0 // to be sure...
+			&& g_atomic_int_compare_and_exchange (&pTask->pThread->iThreadIsRunning, 0, 1)) // we announce that we thread is now running
+		{
+			g_atomic_int_set (&pTask->pThread->iUpdateIsEnded, 0);
+			// the thread is launched but is waiting
+			g_mutex_unlock (CD_TASK_GMUTEX); // unlock to get new data
+		}
 		
-		if (pTask->iSidTimerUpdate == 0)
-			pTask->iSidTimerUpdate = g_timeout_add (MAX (100, MIN (0.10 * pTask->iPeriod, 333)), (GSourceFunc) _cairo_dock_check_for_update, pTask);
+		/* if (pTask->iSidTimerUpdate == 0)
+			pTask->iSidTimerUpdate = g_timeout_add (MAX (100, MIN (0.10 * pTask->iPeriod, 333)), (GSourceFunc) _cairo_dock_check_for_update, pTask);*/
 	}
 }
 
@@ -127,8 +211,10 @@
 	cairo_dock_launch_task (pTask);
 	return FALSE;
 }
+
 void cairo_dock_launch_task_delayed (CairoDockTask *pTask, double fDelay)
 {
+		g_print ("Task: %s %f (%p)\n", __func__, fDelay, pTask);
 	cairo_dock_cancel_next_iteration (pTask);
 	if (fDelay == 0)
 		pTask->iSidTimer = g_idle_add ((GSourceFunc) _cairo_dock_one_shot_timer, pTask);
@@ -155,6 +241,7 @@
 	if (pTask == NULL)
 		return ;
 	
+	g_print ("Task: %s (%p)\n", __func__, pTask);
 	cairo_dock_cancel_next_iteration (pTask);
 	
 	if (pTask->iSidTimerUpdate != 0)
@@ -171,11 +258,11 @@
 	
 	_cairo_dock_pause_task (pTask);
 	
-	cd_message ("***waiting for thread's end...(%d)", g_atomic_int_get (&pTask->iThreadIsRunning));
-	while (g_atomic_int_get (&pTask->iThreadIsRunning))
+	g_print ("***waiting for thread's end...\n"); ///"(%d)\n", g_atomic_int_get (&pTask->iThreadIsRunning));
+	/// while (g_atomic_int_get (&pTask->iThreadIsRunning))
+	while (pTask->pThread)
 		g_usleep (10);
-		///gtk_main_iteration ();
-	cd_message ("***ended.");
+	g_print ("***ended.\n");
 }
 
 
@@ -190,6 +277,7 @@
 	if (pTask == NULL)
 		return ;
 	
+	g_print ("Task: %s (%p)\n", __func__, pTask);
 	cairo_dock_cancel_next_iteration (pTask);
 	g_atomic_int_set (&pTask->bDiscard, 1);
 	
@@ -197,6 +285,15 @@
 		pTask->iSidTimerUpdate = g_idle_add ((GSourceFunc) _free_discarded_task, pTask);
 }
 
+static void _free_task (CairoDockTask *pTask)
+{
+	g_print ("Task: Free task (%p)\n", pTask);
+	if (pTask->free_data)
+		pTask->free_data (pTask->pSharedMemory);
+	g_timer_destroy (pTask->pClock);
+	g_free (pTask);
+}
+
 void cairo_dock_free_task (CairoDockTask *pTask)
 {
 	if (pTask == NULL)
@@ -263,7 +360,7 @@
 			break ;
 		}
 		
-		cd_message ("degradation de la mesure (etat <- %d/%d)", pTask->iFrequencyState, CAIRO_DOCK_NB_FREQUENCIES-1);
+		cd_message ("degradation of the frequency (state <- %d/%d)", pTask->iFrequencyState, CAIRO_DOCK_NB_FREQUENCIES-1);
 		_cairo_dock_restart_timer_with_frequency (pTask, iNewPeriod);
 	}
 }

=== modified file 'src/gldit/cairo-dock-task.h'
--- src/gldit/cairo-dock-task.h	2010-10-31 00:14:40 +0000
+++ src/gldit/cairo-dock-task.h	2012-08-21 14:25:23 +0000
@@ -53,14 +53,27 @@
 /// Definition of the synchronous job, that update the dock with the results of the previous job. Returns TRUE to continue, FALSE to stop
 typedef gboolean (* CairoDockUpdateSyncFunc ) (gpointer pSharedMemory);
 
+typedef struct _CairoDockTaskThread {
+	/// Atomic value, set to 1 when the thread is running.
+	volatile gint iThreadIsRunning;
+	/// Mutex, to know if the thread can receive data
+	#if (GLIB_MAJOR_VERSION == 2 && GLIB_MINOR_VERSION < 32)
+	GMutex *pMutex;
+	#else
+	GMutex aMutex;
+	#endif
+	/// Atomic value, set to 1 when the thread can run
+	volatile gint iThreadCanRun;
+	/// Atomic value, set to 1 when the main-loop is doing the update
+	volatile gint iUpdateIsEnded;
+} CairoDockTaskThread;
+
 /// Definition of a periodic and asynchronous Task.
 struct _CairoDockTask {
 	/// ID of the timer of the Task.
 	gint iSidTimer;
 	/// ID of the timer to check the end of the thread.
 	gint iSidTimerUpdate;
-	/// Atomic value, set to 1 when the thread is running.
-	gint iThreadIsRunning;
 	/// function carrying out the heavy job.
 	CairoDockGetDataAsyncFunc get_data;
 	/// function carrying out the update of the dock. Returns TRUE to continue, FALSE to stop.
@@ -79,6 +92,8 @@
 	GFreeFunc free_data;
 	/// TRUE when the task has been discarded.
 	gboolean bDiscard;
+	/// structure needed to manage threads
+	CairoDockTaskThread *pThread;
 } ;
 
 

=== modified file 'src/implementations/cairo-dock-graph.c'
--- src/implementations/cairo-dock-graph.c	2012-07-19 23:59:06 +0000
+++ src/implementations/cairo-dock-graph.c	2012-08-21 14:25:23 +0000
@@ -40,7 +40,6 @@
 	gdouble fBackGroundColor[4];
 	cairo_surface_t *pBackgroundSurface;
 	GLuint iBackgroundTexture;
-	gint iRadius; // deprecated
 	gint iMargin;
 	gboolean bMixGraphs;
 	} Graph;

=== modified file 'src/implementations/cairo-dock-graph.h'
--- src/implementations/cairo-dock-graph.h	2011-09-13 00:07:51 +0000
+++ src/implementations/cairo-dock-graph.h	2012-08-21 14:25:23 +0000
@@ -58,8 +58,6 @@
 	gdouble *fLowColor;
 	/// color of the background.
 	gdouble fBackGroundColor[4];
-	// radius of the corners of the background.
-	gint iRadius;  // deprecated
 	/// TRUE to draw all the values on the same graph.
 	gboolean bMixGraphs;
 };