GNOME Bugzilla – Bug 670185
[PATCH] Use new GLib threading API
Last modified: 2012-02-19 13:36:58 UTC
Created attachment 207729 [details] [review] Use new GLib threading API Use new GLib threading API I've converted the remainder of EOG to use the new glib threading API. I've attempted to make the code in src/eog-jobs.[ch] match the old API/ABI as I guess plugins will be accessing it? Please check this code is safe.
Review of attachment 207729 [details] [review]: Thanks for patching, Robert. :) The patch looks good to me so far. What I'm going to check over the weekend is if we can leave the g_mutex_clear() calls in the dispose handlers or if we have to move them into a finalize handler. ::: src/eog-jobs.c @@ +67,3 @@ + /* NOTE: We need to allocate the mutex here so the ABI stays the same when it used to use g_mutex_new */ + job->mutex = g_malloc (sizeof (GMutex)); + g_mutex_init (job->mutex); Oh, that's an interesting find. I don't think plugins are supposed to touch that mutex, actually I fail to see the reason for this mutex at all. It's only used to protect the update of the progress value, yet the value is available unprotected (maybe that's the reason the mutex is public?). It's probably safer to keep it that way for now though, but I'm seriously thinking about removing that mutex completely.
(In reply to comment #1) > Review of attachment 207729 [details] [review]: > > Thanks for patching, Robert. :) > > The patch looks good to me so far. > What I'm going to check over the weekend is if we can leave the g_mutex_clear() > calls in the dispose handlers or if we have to move them into a finalize > handler. I made a few tests and it looks like the g_mutex_clear() calls need to go to _finalize as _dispose can be called multiple times. Calling g_mutex_clear multiple times causes a double free and thus aborts the process.
Committed your patch and moved the g_mutex_clear calls into finalize handlers. Strangely git assumed I was the author of your patch (probably due to adding the bug URL to the commit message) and I only noticed that after pushing. Sorry, for that. commit b5f61d90d0478378cf7cf21bf5ade9eded0ae9f2 Author: Felix Riemann <> Date: Sun Feb 19 14:13:40 2012 +0100 Use new GLib threading API https://bugzilla.gnome.org/show_bug.cgi?id=670185 --- This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.