From 90942b6b223d68ad5270aff1fc4a46312cedd208 Mon Sep 17 00:00:00 2001 From: Joe Drew Date: Thu, 21 Mar 2013 23:09:37 -0400 Subject: [PATCH] Bug 853169 - Make qcms precache reference counts threadsafe/atomic. r=jrmuizel --- gfx/qcms/qcms.h | 6 ++++++ gfx/qcms/qcmsint.h | 18 ++++++++++++++++++ gfx/qcms/transform.c | 12 ++++++++++-- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/gfx/qcms/qcms.h b/gfx/qcms/qcms.h index d308d64cf730..dcd74b7ffa9c 100644 --- a/gfx/qcms/qcms.h +++ b/gfx/qcms/qcms.h @@ -40,6 +40,12 @@ sale, use or other dealings in this Software without written authorization from SunSoft Inc. ******************************************************************/ +/* + * QCMS, in general, is not threadsafe. However, it should be safe to create + * profile and transformation objects on different threads, so long as you + * don't use the same objects on different threads at the same time. + */ + /* * Color Space Signatures * Note that only icSigXYZData and icSigLabData are valid diff --git a/gfx/qcms/qcmsint.h b/gfx/qcms/qcmsint.h index 0f033f720021..f630871348c6 100644 --- a/gfx/qcms/qcmsint.h +++ b/gfx/qcms/qcmsint.h @@ -284,6 +284,24 @@ void qcms_transform_data_rgba_out_lut_altivec(qcms_transform *transform, extern qcms_bool qcms_supports_iccv4; +#ifdef _MSC_VER + +long __cdecl _InterlockedIncrement(long volatile *); +long __cdecl _InterlockedDecrement(long volatile *); +#pragma intrinsic(_InterlockedIncrement) +#pragma intrinsic(_InterlockedDecrement) + +#define qcms_atomic_increment(x) _InterlockedIncrement((long volatile *)&x) +#define qcms_atomic_decrement(x) _InterlockedDecrement((long volatile*)&x) + +#else + +#define qcms_atomic_increment(x) __sync_add_and_fetch(&x, 1) +#define qcms_atomic_decrement(x) __sync_sub_and_fetch(&x, 1) + +#endif + + #ifdef NATIVE_OUTPUT # define RGB_OUTPUT_COMPONENTS 4 # define RGBA_OUTPUT_COMPONENTS 4 diff --git a/gfx/qcms/transform.c b/gfx/qcms/transform.c index 87186190539b..785cb7646ea5 100644 --- a/gfx/qcms/transform.c +++ b/gfx/qcms/transform.c @@ -886,9 +886,17 @@ static void qcms_transform_data_rgb_out_linear(qcms_transform *transform, unsign } #endif +/* + * If users create and destroy objects on different threads, even if the same + * objects aren't used on different threads at the same time, we can still run + * in to trouble with refcounts if they aren't atomic. + * + * This can lead to us prematurely deleting the precache if threads get unlucky + * and write the wrong value to the ref count. + */ static struct precache_output *precache_reference(struct precache_output *p) { - p->ref_count++; + qcms_atomic_increment(p->ref_count); return p; } @@ -902,7 +910,7 @@ static struct precache_output *precache_create() void precache_release(struct precache_output *p) { - if (--p->ref_count == 0) { + if (qcms_atomic_decrement(p->ref_count) == 0) { free(p); } }