Bug 853169 - Make qcms precache reference counts threadsafe/atomic. r=jrmuizel
authorJoe Drew <joe@drew.ca>
Wed, 20 Mar 2013 22:25:43 -0400
changeset 125716 eca6b3ea3e8d0d6dde65ed47f9f13811b343c030
parent 125715 e8bfc3e3c24929666c7531d33bfe830ca42b50d4
child 125717 d77773a8abf9c401aa5232fe67f8ebe61f1a6187
push id24461
push useremorley@mozilla.com
push dateThu, 21 Mar 2013 11:51:51 +0000
treeherdermozilla-central@a73a2b5c423b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs853169
milestone22.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 853169 - Make qcms precache reference counts threadsafe/atomic. r=jrmuizel
gfx/qcms/qcms.h
gfx/qcms/qcmsint.h
gfx/qcms/transform.c
--- a/gfx/qcms/qcms.h
+++ b/gfx/qcms/qcms.h
@@ -35,16 +35,22 @@ FROM, OUT OF OR IN CONNECTION WITH THE S
 OTHER DEALINGS IN THE SOFTWARE. 
  
 Except as contained in this notice, the name of SunSoft, Inc. 
 shall not be used in advertising or otherwise to promote the 
 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
  * Profile Connection Spaces (PCSs)
  */ 
 typedef enum {
     icSigXYZData                        = 0x58595A20L,  /* 'XYZ ' */
     icSigLabData                        = 0x4C616220L,  /* 'Lab ' */
--- a/gfx/qcms/qcmsint.h
+++ b/gfx/qcms/qcmsint.h
@@ -279,16 +279,34 @@ void qcms_transform_data_rgb_out_lut_alt
                                              size_t length);
 void qcms_transform_data_rgba_out_lut_altivec(qcms_transform *transform,
                                               unsigned char *src,
                                               unsigned char *dest,
                                               size_t length);
 
 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
 # ifdef IS_LITTLE_ENDIAN
 #  define OUTPUT_A_INDEX 3
 #  define OUTPUT_R_INDEX 2
 #  define OUTPUT_G_INDEX 1
 #  define OUTPUT_B_INDEX 0
--- a/gfx/qcms/transform.c
+++ b/gfx/qcms/transform.c
@@ -881,33 +881,41 @@ static void qcms_transform_data_rgb_out_
 
 		*dest++ = clamp_u8(out_linear_r*255);
 		*dest++ = clamp_u8(out_linear_g*255);
 		*dest++ = clamp_u8(out_linear_b*255);
 	}
 }
 #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;
 }
 
 static struct precache_output *precache_create()
 {
 	struct precache_output *p = malloc(sizeof(struct precache_output));
 	if (p)
 		p->ref_count = 1;
 	return p;
 }
 
 void precache_release(struct precache_output *p)
 {
-	if (--p->ref_count == 0) {
+	if (qcms_atomic_decrement(p->ref_count) == 0) {
 		free(p);
 	}
 }
 
 #ifdef HAS_POSIX_MEMALIGN
 static qcms_transform *transform_alloc(void)
 {
 	qcms_transform *t;