Search code examples
cmultithreadinggtk3glibgdk

How do I avoid a race condition at the end of a GTask that updates a GTK UI?


I'm building a gtk3 application and was trying to figure out to avoid a race condition at the completion of a GTask that updates the UI.

As the function (data_acq()) called by the GTask is long-running, I have a progress bar that updates (via gdk_threads_add_timeout(progressBar_timeout_cb)), but at the end of data_acq() I use gdk_threads_add_idle(progressBar_complete, params) to set the progress bar to 100%. The issue arises in that the last step of the GTask is to automatically call free_data_acq_data(params) which frees the pointer to the progress bar, causing a segfault when progressBar_complete(params) triggers, if it happens after free_data_acq_data(params) happens.

It's possible that this issue is entirely due to my abuse of pointers (or some other rookie mistake), but I admit that I'm not seeing how to pass the data in a manner that guarantees 1) that it will be freed after use and 2) that it will not be freed too early. Note that it is possible that the task is cancelled and so I don't want to free the memory in progressBar_complete().

So my question(s): is there a better way to avoid a race condition in this case (or is there another way to structure the code to avoid this issue)? Or is there a mechanism for checking if a gdk_threads_add_idle() function has completed so I can tell the memory-freeing function to wait until the idle is done?

I tried using a global to track if progressBar_complete had run or not, but I couldn't get it to play nicely with the GTask because it meant that free_data_acq_data never completed (as the thread was never relinquished to progressBar_complete to update the global).

For concreteness, here is some example code:

#include <gtk/gtk.h>

struct dataAcqParams {
  GtkWidget *progressBar;
  int timeoutID;
  GCancellable *cancellable;
};


void update_progressBar(GtkWidget *progressBar,
                        double fraction)
{
  gtk_progress_bar_set_fraction(GTK_PROGRESS_BAR(progressBar), fraction);
}

int complete_progressBar(gpointer data)
{
  struct dataAcqParams *params = data;
  GtkWidget *progressBar;
  progressBar = params->progressBar;
  g_source_remove(params->timeoutID); // This turns off automatic progress bar updates
  update_progressBar(progressBar, 1.0);

  return G_SOURCE_REMOVE;
}

int progressBar_timeout_cb(gpointer data)
{
  GtkWidget *progressBar;
  progressBar = params->progressBar;

  double fraction = 0.50; // For brevity
  update_progressBar(progressBar, fraction);

  return G_SOURCE_CONTINUE; // We keep calling this until we cancel it...
}

void free_data_acq_data(void *data)
{
  struct dataAcqParams *params = data;
  g_free(params);
}

int data_acq(struct dataAcqParams *data)
{
  struct dataAcqParams *params = data;

  g_print("Performing a measurement...\n");
  g_usleep(10e5); // Some long computation, can be cancelled
    
  gdk_threads_add_idle(complete_progressBar, params); // Race condition starts here
  g_usleep(500000); // Delay to prevent race condition, surely there is a better way?

  return 0;
}

static void data_acq_cb(GTask    *task,
                        gpointer source_object,
                        gpointer task_data,
                        GCancellable *cancellable)
{
  struct dataAcqParams *params = task_data;
  int retval;

  // Handle Cancellation:
  if(g_task_return_error_if_cancelled(task))
  {
    return;
  }

  retval = data_acq(params);

  g_task_return_int(task, retval);
}

void start_data_acq_async(gpointer            data,
                          GCancellable       *cancellable,
                          GAsyncReadyCallback callback,
                          gpointer            user_data)
{

  GTask *task = NULL;
  struct dataAcqParams *params;
  params = (struct dataAcqParams *) data;

  // Error if this is badly formatted:
  g_return_if_fail(cancellable == NULL | G_IS_CANCELLABLE(cancellable));

  task = g_task_new(NULL, cancellable, callback, user_data);
  g_task_set_source_tag(task, start_data_acq_async);

  g_task_set_return_on_cancel(task, FALSE);

  g_task_set_task_data(task, params, free_data_acq_data);

  // Run the acquisition in a worker thread:
  g_task_run_in_thread(task, data_acq_cb);

  g_object_unref(task);
}

int main(int argc, char **argv)
{
  gtk_init(&argc, &argv);
  
  // ... build/show ui ...
  GtkWidget *progressBar;
  GtkBuilder *builder;
  // ...

  progressBar = GTK_WIDGET(gtk_builder_get_object(builder, "progress_bar"));
  GCancellable *cancellable; 
 
  struct dataAcqParams *params = g_malloc(sizeof(*params));

  params->progressBar = progressBar;
  params->cancellable = cancellable;
  params->timeoutID = gdk_threads_add_timeout(100, progressBar_timeout_cb,
                                        params);

  start_data_acq_async(params, cancellable, NULL, NULL);

  gtk_main();

  return 0;
}

Solution

  • If you need to access the progress bar pointer from the UI thread, then I would suggest not freeing it in the worker thread. Maybe when the task completes or is cancelled, you could use gdk_threads_add_idle() to queue up another function that will free the resources used on the UI side like dataAcqParams, instead of freeing it in the worker thread?