Suppress data races in SkEventTracer

Suppress data races in SkEventTracer

BUG=437044
TBR=mtklein@chromium.org

Committed: https://crrev.com/f0a9e71e6ad0b13397d45afeeeba7c0ced15b570
Cr-Commit-Position: refs/heads/master@{#326024}

[Alexander Potapenko, 2015-04-21 13:36:52]

TBR

[Alexander Potapenko, 2015-04-21 13:36:56]

The CQ bit was checked by glider@chromium.org

[commit-bot: I haz the power, 2015-04-21 13:37:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1081863003/1

[mtklein, 2015-04-21 13:50:52]

mtklein@google.com changed reviewers:
+ mtklein@google.com

[mtklein, 2015-04-21 13:50:52]

It's not clear suppressing this is a good idea.  Isn't a race on SetInstance a
potential source of crashes, deleting the existing tracer while it's in use?

[commit-bot: I haz the power, 2015-04-21 13:57:51]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-04-21 13:59:03]

Patchset 1 (id:??) landed as
https://crrev.com/f0a9e71e6ad0b13397d45afeeeba7c0ced15b570
Cr-Commit-Position: refs/heads/master@{#326024}

[Alexander Potapenko, 2015-04-21 14:03:36]

On 2015/04/21 13:50:52, mtklein wrote:
> It's not clear suppressing this is a good idea.  Isn't a race on SetInstance a
> potential source of crashes, deleting the existing tracer while it's in use?

This is the memory sheriffing workflow :)
We need to make the waterfall green, thus we need to suppress the bugs when it's
hard to revert.
This doesn't necessarily mean the race is benign and needs not to be fixed.
It's the responsibility of the code author to fix the bug and remove the
suppression.

[mtklein_C, 2015-04-27 13:37:10]

On 2015/04/21 14:03:36, Alexander Potapenko wrote:
> On 2015/04/21 13:50:52, mtklein wrote:
> > It's not clear suppressing this is a good idea.  Isn't a race on SetInstance
a
> > potential source of crashes, deleting the existing tracer while it's in use?
> 
> This is the memory sheriffing workflow :)
> We need to make the waterfall green, thus we need to suppress the bugs when
it's
> hard to revert.
> This doesn't necessarily mean the race is benign and needs not to be fixed.
> It's the responsibility of the code author to fix the bug and remove the
> suppression.

So I think we fixed this last week.  What's the workflow to determine if we have
and unsuppress it?  Is is, create a revert of this CL, run a Linux TSAN Tests
trybot, submit if it passes?

[Alexander Potapenko, 2015-04-27 16:50:06]

On 2015/04/27 13:37:10, mtklein_C wrote:
> On 2015/04/21 14:03:36, Alexander Potapenko wrote:
> > On 2015/04/21 13:50:52, mtklein wrote:
> > > It's not clear suppressing this is a good idea.  Isn't a race on
SetInstance
> a
> > > potential source of crashes, deleting the existing tracer while it's in
use?
> > 
> > This is the memory sheriffing workflow :)
> > We need to make the waterfall green, thus we need to suppress the bugs when
> it's
> > hard to revert.
> > This doesn't necessarily mean the race is benign and needs not to be fixed.
> > It's the responsibility of the code author to fix the bug and remove the
> > suppression.
> 
> So I think we fixed this last week.  What's the workflow to determine if we
have
> and unsuppress it?  Is is, create a revert of this CL, run a Linux TSAN Tests
> trybot, submit if it passes?

Yeah, that should be enough.

[mtklein, 2015-04-27 17:49:46]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1111433002/ by mtklein@google.com.

The reason for reverting is: I think this is fixed..