From 4f9db59bfd813196e49b3d0beae51d2fe84f46ec Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Tue, 16 Nov 2004 02:51:19 +0000 Subject: [PATCH] Be extra careful when locking and handle the case that the module may 2004-11-15 Matthias Clasen * gdk-pixbuf-animation.c: * gdk-pixbuf-loader.c: * gdk-pixbuf-io.c: Be extra careful when locking and handle the case that the module may initialize the thread system. * gdk-pixbuf-io.[hc] (_gdk_pixbuf_lock): Return whether the lock was actually taken. --- gdk-pixbuf/ChangeLog | 8 +++++ gdk-pixbuf/gdk-pixbuf-animation.c | 6 ++-- gdk-pixbuf/gdk-pixbuf-io.c | 59 ++++++++++++++++++++++--------- gdk-pixbuf/gdk-pixbuf-loader.c | 6 ++-- 4 files changed, 56 insertions(+), 23 deletions(-) diff --git a/gdk-pixbuf/ChangeLog b/gdk-pixbuf/ChangeLog index 786dd035b5..454881041d 100644 --- a/gdk-pixbuf/ChangeLog +++ b/gdk-pixbuf/ChangeLog @@ -1,5 +1,13 @@ 2004-11-15 Matthias Clasen + * gdk-pixbuf-animation.c: + * gdk-pixbuf-loader.c: + * gdk-pixbuf-io.c: Be extra careful when locking and handle + the case that the module may initialize the thread system. + + * gdk-pixbuf-io.[hc] (_gdk_pixbuf_lock): Return whether the + lock was actually taken. + * queryloaders.c (query_module): Set vtable->module before calling fill_vtable(), since gdk-pixbuf-io.c does does the same and modules may rely on it. (#158177, Dan Winship) diff --git a/gdk-pixbuf/gdk-pixbuf-animation.c b/gdk-pixbuf/gdk-pixbuf-animation.c index fa6968c153..56e5864e92 100644 --- a/gdk-pixbuf/gdk-pixbuf-animation.c +++ b/gdk-pixbuf/gdk-pixbuf-animation.c @@ -137,6 +137,7 @@ gdk_pixbuf_animation_new_from_file (const char *filename, guchar buffer [128]; GdkPixbufModule *image_module; gchar *display_name; + gboolean locked; g_return_val_if_fail (filename != NULL, NULL); g_return_val_if_fail (error == NULL || *error == NULL, NULL); @@ -181,7 +182,7 @@ gdk_pixbuf_animation_new_from_file (const char *filename, return NULL; } - _gdk_pixbuf_lock (image_module); + locked = _gdk_pixbuf_lock (image_module); if (image_module->load_animation == NULL) { GdkPixbuf *pixbuf; @@ -245,7 +246,8 @@ gdk_pixbuf_animation_new_from_file (const char *filename, g_free (display_name); out_unlock: - _gdk_pixbuf_unlock (image_module); + if (locked) + _gdk_pixbuf_unlock (image_module); return animation; } diff --git a/gdk-pixbuf/gdk-pixbuf-io.c b/gdk-pixbuf/gdk-pixbuf-io.c index 5c2157e3ea..02d4eed615 100644 --- a/gdk-pixbuf/gdk-pixbuf-io.c +++ b/gdk-pixbuf/gdk-pixbuf-io.c @@ -79,12 +79,17 @@ format_check (GdkPixbufModule *module, guchar *buffer, int size) G_LOCK_DEFINE_STATIC (init_lock); G_LOCK_DEFINE_STATIC (threadunsafe_loader_lock); -void +gboolean _gdk_pixbuf_lock (GdkPixbufModule *image_module) { - if (!(image_module->info->flags & GDK_PIXBUF_FORMAT_THREADSAFE)) { + if (g_threads_got_initialized && + !(image_module->info->flags & GDK_PIXBUF_FORMAT_THREADSAFE)) { G_LOCK (threadunsafe_loader_lock); + + return TRUE; } + + return FALSE; } void @@ -460,10 +465,19 @@ _gdk_pixbuf_load_module (GdkPixbufModule *image_module, GError **error) { gboolean ret; + gboolean locked = FALSE; - G_LOCK (init_lock); + /* be extra careful, maybe the module initializes + * the thread system + */ + if (g_threads_got_initialized) + { + G_LOCK (init_lock); + locked = TRUE; + } ret = _gdk_pixbuf_load_module_unlocked (image_module, error); - G_UNLOCK (init_lock); + if (locked) + G_UNLOCK (init_lock); return ret; } @@ -736,8 +750,9 @@ _gdk_pixbuf_generic_image_load (GdkPixbufModule *module, GdkPixbuf *pixbuf = NULL; GdkPixbufAnimation *animation = NULL; gpointer context; + gboolean locked; - _gdk_pixbuf_lock (module); + locked = _gdk_pixbuf_lock (module); if (module->load != NULL) { pixbuf = (* module->load) (f, error); @@ -773,13 +788,13 @@ _gdk_pixbuf_generic_image_load (GdkPixbufModule *module, pixbuf = gdk_pixbuf_animation_get_static_image (animation); g_object_ref (pixbuf); - g_object_unref (animation); } } out: - _gdk_pixbuf_unlock (module); + if (locked) + _gdk_pixbuf_unlock (module); return pixbuf; } @@ -1154,7 +1169,10 @@ gdk_pixbuf_new_from_xpm_data (const char **data) GdkPixbuf *(* load_xpm_data) (const char **data); GdkPixbuf *pixbuf; GError *error = NULL; - GdkPixbufModule *xpm_module = _gdk_pixbuf_get_named_module ("xpm", &error); + GdkPixbufModule *xpm_module; + gboolean locked; + + xpm_module = _gdk_pixbuf_get_named_module ("xpm", &error); if (xpm_module == NULL) { g_warning ("Error loading XPM image loader: %s", error->message); g_error_free (error); @@ -1169,7 +1187,7 @@ gdk_pixbuf_new_from_xpm_data (const char **data) } } - _gdk_pixbuf_lock (xpm_module); + locked = _gdk_pixbuf_lock (xpm_module); if (xpm_module->load_xpm_data == NULL) { g_warning ("gdk-pixbuf XPM module lacks XPM data capability"); @@ -1178,8 +1196,9 @@ gdk_pixbuf_new_from_xpm_data (const char **data) load_xpm_data = xpm_module->load_xpm_data; pixbuf = (* load_xpm_data) (data); } - - _gdk_pixbuf_unlock (xpm_module); + + if (locked) + _gdk_pixbuf_unlock (xpm_module); return pixbuf; } @@ -1250,6 +1269,7 @@ gdk_pixbuf_real_save (GdkPixbuf *pixbuf, { gboolean ret; GdkPixbufModule *image_module = NULL; + gboolean locked; image_module = _gdk_pixbuf_get_named_module (type, error); @@ -1260,7 +1280,7 @@ gdk_pixbuf_real_save (GdkPixbuf *pixbuf, if (!_gdk_pixbuf_load_module (image_module, error)) return FALSE; - _gdk_pixbuf_lock (image_module); + locked = _gdk_pixbuf_lock (image_module); if (image_module->save) { /* save normally */ @@ -1283,7 +1303,8 @@ gdk_pixbuf_real_save (GdkPixbuf *pixbuf, ret = FALSE; } - _gdk_pixbuf_unlock (image_module); + if (locked) + _gdk_pixbuf_unlock (image_module); return ret; } @@ -1304,6 +1325,7 @@ save_to_callback_with_tmp_file (GdkPixbufModule *image_module, gchar *buf = NULL; gsize n; gchar *filename = NULL; + gboolean locked; buf = g_try_malloc (TMP_FILE_BUF_SIZE); if (buf == NULL) { @@ -1326,9 +1348,10 @@ save_to_callback_with_tmp_file (GdkPixbufModule *image_module, goto end; } - _gdk_pixbuf_lock (image_module); + locked = _gdk_pixbuf_lock (image_module); retval = (image_module->save) (f, pixbuf, keys, values, error); - _gdk_pixbuf_unlock (image_module); + if (locked) + _gdk_pixbuf_unlock (image_module); if (!retval) goto end; @@ -1375,6 +1398,7 @@ gdk_pixbuf_real_save_to_callback (GdkPixbuf *pixbuf, { gboolean ret; GdkPixbufModule *image_module = NULL; + gboolean locked; image_module = _gdk_pixbuf_get_named_module (type, error); @@ -1385,7 +1409,7 @@ gdk_pixbuf_real_save_to_callback (GdkPixbuf *pixbuf, if (!_gdk_pixbuf_load_module (image_module, error)) return FALSE; - _gdk_pixbuf_lock (image_module); + locked = _gdk_pixbuf_lock (image_module); if (image_module->save_to_callback) { /* save normally */ @@ -1408,7 +1432,8 @@ gdk_pixbuf_real_save_to_callback (GdkPixbuf *pixbuf, ret = FALSE; } - _gdk_pixbuf_unlock (image_module); + if (locked) + _gdk_pixbuf_unlock (image_module); return ret; } diff --git a/gdk-pixbuf/gdk-pixbuf-loader.c b/gdk-pixbuf/gdk-pixbuf-loader.c index 0ce63605d3..9e252c5706 100644 --- a/gdk-pixbuf/gdk-pixbuf-loader.c +++ b/gdk-pixbuf/gdk-pixbuf-loader.c @@ -387,8 +387,7 @@ gdk_pixbuf_loader_load_module (GdkPixbufLoader *loader, } if (!priv->holds_threadlock) { - _gdk_pixbuf_lock (priv->image_module); - priv->holds_threadlock = TRUE; + priv->holds_threadlock = _gdk_pixbuf_lock (priv->image_module); } priv->context = priv->image_module->begin_load (gdk_pixbuf_loader_size_func, @@ -745,8 +744,7 @@ gdk_pixbuf_loader_close (GdkPixbufLoader *loader, } priv->closed = TRUE; - if (priv->image_module) { - g_assert (priv->holds_threadlock); + if (priv->image_module && priv->holds_threadlock) { _gdk_pixbuf_unlock (priv->image_module); priv->holds_threadlock = FALSE; }