Bug 853169 - Make qcms precache reference counts threadsafe/atomic. r=jrmuizel
authorJoe Drew <joe@drew.ca>
Thu, 21 Mar 2013 23:09:37 -0400
changeset 125913 8987af28db8f623cf706711ee9145baa94419335
parent 125912 4e743f7bf1141e4dc1d28f8c94e4f18ded8bc2a6
child 125914 a92fbe6fd787e532c823a615c48c1758a9b79cf8
push id24464
push useremorley@mozilla.com
push dateFri, 22 Mar 2013 14:00:12 +0000
treeherdermozilla-central@3825fdbcec62 [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;