Fix 3 related races in SkColorSpace.cpp.

As written one thread can be comparing against, say, gSRGB, while
another thread is initializing it.  TSAN caught us.

To fix this, we funnel everything through a function that returns its
own local static, which is intialized on first use in a thread safe way.

Change-Id: I2b7aa4628daff0511ad969d9800d40d967e2938e
Reviewed-on: https://skia-review.googlesource.com/48761
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
This commit is contained in:
Mike Klein 2017-09-19 16:12:29 -04:00 committed by Skia Commit-Bot
parent 76399d1b4f
commit 6c4ea7e880

View File

@ -9,7 +9,6 @@
#include "SkColorSpace_Base.h" #include "SkColorSpace_Base.h"
#include "SkColorSpace_XYZ.h" #include "SkColorSpace_XYZ.h"
#include "SkColorSpacePriv.h" #include "SkColorSpacePriv.h"
#include "SkOnce.h"
#include "SkPoint3.h" #include "SkPoint3.h"
bool SkColorSpacePrimaries::toXYZD50(SkMatrix44* toXYZ_D50) const { bool SkColorSpacePrimaries::toXYZD50(SkMatrix44* toXYZ_D50) const {
@ -195,51 +194,32 @@ sk_sp<SkColorSpace> SkColorSpace::MakeRGB(const SkColorSpaceTransferFn& coeffs,
return SkColorSpace::MakeRGB(coeffs, toXYZD50); return SkColorSpace::MakeRGB(coeffs, toXYZD50);
} }
static SkColorSpace* gAdobeRGB; static SkColorSpace* singleton_colorspace(SkGammaNamed gamma, const float to_xyz[9]) {
static SkColorSpace* gSRGB; SkMatrix44 m44(SkMatrix44::kUninitialized_Constructor);
static SkColorSpace* gSRGBLinear; m44.set3x3RowMajorf(to_xyz);
(void)m44.getType(); // Force typemask to be computed to avoid races.
return new SkColorSpace_XYZ(gamma, m44);
}
static SkColorSpace* adobe_rgb() {
static SkColorSpace* cs = singleton_colorspace(k2Dot2Curve_SkGammaNamed, gAdobeRGB_toXYZD50);
return cs;
}
static SkColorSpace* srgb() {
static SkColorSpace* cs = singleton_colorspace(kSRGB_SkGammaNamed, gSRGB_toXYZD50);
return cs;
}
static SkColorSpace* srgb_linear() {
static SkColorSpace* cs = singleton_colorspace(kLinear_SkGammaNamed, gSRGB_toXYZD50);
return cs;
}
sk_sp<SkColorSpace> SkColorSpace_Base::MakeNamed(Named named) { sk_sp<SkColorSpace> SkColorSpace_Base::MakeNamed(Named named) {
static SkOnce sRGBOnce;
static SkOnce adobeRGBOnce;
static SkOnce sRGBLinearOnce;
switch (named) { switch (named) {
case kSRGB_Named: { case kSRGB_Named: return sk_ref_sp(srgb());
sRGBOnce([] { case kAdobeRGB_Named: return sk_ref_sp(adobe_rgb());
SkMatrix44 srgbToxyzD50(SkMatrix44::kUninitialized_Constructor); case kSRGBLinear_Named: return sk_ref_sp(srgb_linear());
srgbToxyzD50.set3x3RowMajorf(gSRGB_toXYZD50); default: break;
// Force the mutable type mask to be computed. This avoids races.
(void)srgbToxyzD50.getType();
gSRGB = new SkColorSpace_XYZ(kSRGB_SkGammaNamed, srgbToxyzD50);
});
return sk_ref_sp<SkColorSpace>(gSRGB);
}
case kAdobeRGB_Named: {
adobeRGBOnce([] {
SkMatrix44 adobergbToxyzD50(SkMatrix44::kUninitialized_Constructor);
adobergbToxyzD50.set3x3RowMajorf(gAdobeRGB_toXYZD50);
// Force the mutable type mask to be computed. This avoids races.
(void)adobergbToxyzD50.getType();
gAdobeRGB = new SkColorSpace_XYZ(k2Dot2Curve_SkGammaNamed, adobergbToxyzD50);
});
return sk_ref_sp<SkColorSpace>(gAdobeRGB);
}
case kSRGBLinear_Named: {
sRGBLinearOnce([] {
SkMatrix44 srgbToxyzD50(SkMatrix44::kUninitialized_Constructor);
srgbToxyzD50.set3x3RowMajorf(gSRGB_toXYZD50);
// Force the mutable type mask to be computed. This avoids races.
(void)srgbToxyzD50.getType();
gSRGBLinear = new SkColorSpace_XYZ(kLinear_SkGammaNamed, srgbToxyzD50);
});
return sk_ref_sp<SkColorSpace>(gSRGBLinear);
}
default:
break;
} }
return nullptr; return nullptr;
} }
@ -277,7 +257,7 @@ bool SkColorSpace::toXYZD50(SkMatrix44* toXYZD50) const {
} }
bool SkColorSpace::isSRGB() const { bool SkColorSpace::isSRGB() const {
return gSRGB == this; return srgb() == this;
} }
/////////////////////////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////////////////////////////
@ -344,19 +324,19 @@ size_t SkColorSpace::writeToMemory(void* memory) const {
const SkColorSpace_XYZ* thisXYZ = static_cast<const SkColorSpace_XYZ*>(this); const SkColorSpace_XYZ* thisXYZ = static_cast<const SkColorSpace_XYZ*>(this);
// If we have a named profile, only write the enum. // If we have a named profile, only write the enum.
const SkGammaNamed gammaNamed = thisXYZ->gammaNamed(); const SkGammaNamed gammaNamed = thisXYZ->gammaNamed();
if (this == gSRGB) { if (this == srgb()) {
if (memory) { if (memory) {
*((ColorSpaceHeader*) memory) = ColorSpaceHeader::Pack( *((ColorSpaceHeader*) memory) = ColorSpaceHeader::Pack(
k0_Version, SkColorSpace_Base::kSRGB_Named, gammaNamed, 0); k0_Version, SkColorSpace_Base::kSRGB_Named, gammaNamed, 0);
} }
return sizeof(ColorSpaceHeader); return sizeof(ColorSpaceHeader);
} else if (this == gAdobeRGB) { } else if (this == adobe_rgb()) {
if (memory) { if (memory) {
*((ColorSpaceHeader*) memory) = ColorSpaceHeader::Pack( *((ColorSpaceHeader*) memory) = ColorSpaceHeader::Pack(
k0_Version, SkColorSpace_Base::kAdobeRGB_Named, gammaNamed, 0); k0_Version, SkColorSpace_Base::kAdobeRGB_Named, gammaNamed, 0);
} }
return sizeof(ColorSpaceHeader); return sizeof(ColorSpaceHeader);
} else if (this == gSRGBLinear) { } else if (this == srgb_linear()) {
if (memory) { if (memory) {
*((ColorSpaceHeader*) memory) = ColorSpaceHeader::Pack( *((ColorSpaceHeader*) memory) = ColorSpaceHeader::Pack(
k0_Version, SkColorSpace_Base::kSRGBLinear_Named, gammaNamed, 0); k0_Version, SkColorSpace_Base::kSRGBLinear_Named, gammaNamed, 0);