summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlexander Larsson <alexl@redhat.com>2014-07-30 12:32:21 +0200
committerAlexander Larsson <alexl@redhat.com>2014-07-30 15:11:01 +0200
commitb1dd594a22e3499caafdeccd7fa223a032b9e177 (patch)
treef7a172ca350816ab4e98d555fe8b1a2469d49ed6
parent2266f6b743a5a4e4d560238ebd085bf0e1ad8934 (diff)
Remove atomics from g_clear_object/g_clear_pointer
Practically no caller of these functions require atomic behaviour, but the atomics are much slower than normal operations, which makes it desirable to get rid of them. We have not done this before because that would be a break of the ABI. However, I recently looked into this and it seems that even if the atomics *are* used for g_clear_* it is not ever safe to use this. The atomics protects two threads that are racing to free a global/shared object from freeing the object twice. However, any *user* of the global object have no protection from the object being freed while in use, because there is no paired operation the reads and refs the object as an atomic unit (nor can such an operation be implemented using purely atomic ops). So, since nothing could safely have used the atomic aspects of these functions I consider it acceptable to just remove it. https://bugzilla.gnome.org/show_bug.cgi?id=733969
-rw-r--r--glib/gmem.c16
-rw-r--r--glib/gmem.h13
-rw-r--r--gobject/gobject.c3
3 files changed, 11 insertions, 21 deletions
diff --git a/glib/gmem.c b/glib/gmem.c
index 5363751f2..de08b335c 100644
--- a/glib/gmem.c
+++ b/glib/gmem.c
@@ -204,9 +204,6 @@ g_free (gpointer mem)
* Otherwise, the variable is destroyed using @destroy and the
* pointer is set to %NULL.
*
- * This function is threadsafe and modifies the pointer atomically,
- * using memory barriers where needed.
- *
* A macro is also included that allows this function to be used without
* pointer casts.
*
@@ -219,15 +216,12 @@ g_clear_pointer (gpointer *pp,
{
gpointer _p;
- /* This is a little frustrating.
- * Would be nice to have an atomic exchange (with no compare).
- */
- do
- _p = g_atomic_pointer_get (pp);
- while G_UNLIKELY (!g_atomic_pointer_compare_and_exchange (pp, _p, NULL));
-
+ _p = *pp;
if (_p)
- destroy (_p);
+ {
+ *pp = NULL;
+ destroy (_p);
+ }
}
/**
diff --git a/glib/gmem.h b/glib/gmem.h
index e1d88d5a1..5dcc3f891 100644
--- a/glib/gmem.h
+++ b/glib/gmem.h
@@ -117,13 +117,12 @@ gpointer g_try_realloc_n (gpointer mem,
/* This assignment is needed to avoid a gcc warning */ \
GDestroyNotify _destroy = (GDestroyNotify) (destroy); \
\
- (void) (0 ? (gpointer) *(pp) : 0); \
- do \
- _p = g_atomic_pointer_get (_pp); \
- while G_UNLIKELY (!g_atomic_pointer_compare_and_exchange (_pp, _p, NULL)); \
- \
- if (_p) \
- _destroy (_p); \
+ _p = *_pp; \
+ if (_p) \
+ { \
+ *_pp = NULL; \
+ _destroy (_p); \
+ } \
} G_STMT_END
/* Optimise: avoid the call to the (slower) _n function if we can
diff --git a/gobject/gobject.c b/gobject/gobject.c
index 66af56678..cd00244f6 100644
--- a/gobject/gobject.c
+++ b/gobject/gobject.c
@@ -3197,9 +3197,6 @@ g_object_unref (gpointer _object)
* Otherwise, the reference count of the object is decreased and the
* pointer is set to %NULL.
*
- * This function is threadsafe and modifies the pointer atomically,
- * using memory barriers where needed.
- *
* A macro is also included that allows this function to be used without
* pointer casts.
*