Bug 853169 - Make qcms precache reference counts threadsafe/atomic. r=jrmuizel
☠☠ backed out by 7f00c15c1d55 ☠ ☠
authorJoe Drew <joe@drew.ca>
Thu, 21 Mar 2013 23:09:37 -0400
changeset 125903 2e318d0a172bea34c42dfbab1b91359731c2635e
parent 125902 69f965c0fd4640f8588a241724e8167b70eb4d83
child 125904 c0befbf2533dee1893981a234d4731c442f70568
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;