tracing: Fix AddMetadataEvent race in presence of Convertable arguments

tracing: Fix AddMetadataEvent race in presence of Convertable arguments

crrev.com/1421053002 introduced a TraceLog::AddMetadataEvent method.
This method has a little race, unveiled by crbug.com/559117.
The race happens only when a ConvertableToTraceFormat is passed to
AddMetadataEvent. Explanation of the race:
 - AddMetadataEvent takes optional ConvertableToTraceFormat args.
 - ConvertableToTraceFormat is RefCounted but not RCThreadSafe.
 - AddMetadataEvent acquires the global TraceLog lock_ (which is good)
   but does that only after having initialized the trace_event.
 - CTTF args are copied into the TraceEvent outside of the TraceLog
   lock. The underlying CTTF AddRef/Deref happen outside the lock.
 - Later, when we get to AddMetadataEventsWhileLocked we try to
   re-AddRef the CTTF args.
At this point TSAN barks because the CTTF refcount is written both
ouside and inside the lock.
The fix is trivial, just anticipate the lock of AddMetadataEvent by
one statement.

BUG=559117
TBR=oysteine@chromium.org

Committed: https://crrev.com/5c0f93b3235b44f0c69b23063fa10e49da067666
Cr-Commit-Position: refs/heads/master@{#369401}

[Primiano Tucci (use gerrit), 2016-01-14 11:19:37]

primiano@chromium.org changed reviewers:
+ oysteine@chromium.org, ruuda@google.com

[Primiano Tucci (use gerrit), 2016-01-14 11:19:37]

1 liner

[Ruud van Asseldonk, 2016-01-14 11:27:31]

On 2016/01/14 11:19:37, Primiano Tucci wrote:
> 1 liner

LGTM with nit:

> The fix is trivial, just anticipate the lock of AddMetadataEvent by
> one instruction.

/s/instruction/statement/

[Primiano Tucci (use gerrit), 2016-01-14 11:27:53]

Description was changed from

==========
tracing: Fix AddMetadataEvent race in presence of Convertable arguments

crrev.com/1421053002 introduced a TraceLog::AddMetadataEvent method.
This method has a little race, unveiled by crbug.com/559117.
The race happens only when a ConvertableToTraceFormat is passed to
AddMetadataEvent. Explanation of the race:
 - AddMetadataEvent takes optional ConvertableToTraceFormat args.
 - ConvertableToTraceFormat is RefCounted but not RCThreadSafe.
 - AddMetadataEvent acquires the global TraceLog lock_ (which is good)
   but does that only after having initialized the trace_event.
 - CTTF args are copied into the TraceEvent outside of the TraceLog
   lock. The underlying CTTF AddRef/Deref happen outside the lock.
 - Later, when we get to AddMetadataEventsWhileLocked we try to
   re-AddRef the CTTF args.
At this point TSAN barks because the CTTF refcount is written both
ouside and inside the lock.
The fix is trivial, just anticipate the lock of AddMetadataEvent by
one instruction.

BUG=559117
==========

to

==========
tracing: Fix AddMetadataEvent race in presence of Convertable arguments

crrev.com/1421053002 introduced a TraceLog::AddMetadataEvent method.
This method has a little race, unveiled by crbug.com/559117.
The race happens only when a ConvertableToTraceFormat is passed to
AddMetadataEvent. Explanation of the race:
 - AddMetadataEvent takes optional ConvertableToTraceFormat args.
 - ConvertableToTraceFormat is RefCounted but not RCThreadSafe.
 - AddMetadataEvent acquires the global TraceLog lock_ (which is good)
   but does that only after having initialized the trace_event.
 - CTTF args are copied into the TraceEvent outside of the TraceLog
   lock. The underlying CTTF AddRef/Deref happen outside the lock.
 - Later, when we get to AddMetadataEventsWhileLocked we try to
   re-AddRef the CTTF args.
At this point TSAN barks because the CTTF refcount is written both
ouside and inside the lock.
The fix is trivial, just anticipate the lock of AddMetadataEvent by
one statement.

BUG=559117
==========

[Primiano Tucci (use gerrit), 2016-01-14 11:51:52]

Description was changed from

==========
tracing: Fix AddMetadataEvent race in presence of Convertable arguments

crrev.com/1421053002 introduced a TraceLog::AddMetadataEvent method.
This method has a little race, unveiled by crbug.com/559117.
The race happens only when a ConvertableToTraceFormat is passed to
AddMetadataEvent. Explanation of the race:
 - AddMetadataEvent takes optional ConvertableToTraceFormat args.
 - ConvertableToTraceFormat is RefCounted but not RCThreadSafe.
 - AddMetadataEvent acquires the global TraceLog lock_ (which is good)
   but does that only after having initialized the trace_event.
 - CTTF args are copied into the TraceEvent outside of the TraceLog
   lock. The underlying CTTF AddRef/Deref happen outside the lock.
 - Later, when we get to AddMetadataEventsWhileLocked we try to
   re-AddRef the CTTF args.
At this point TSAN barks because the CTTF refcount is written both
ouside and inside the lock.
The fix is trivial, just anticipate the lock of AddMetadataEvent by
one statement.

BUG=559117
==========

to

==========
tracing: Fix AddMetadataEvent race in presence of Convertable arguments

crrev.com/1421053002 introduced a TraceLog::AddMetadataEvent method.
This method has a little race, unveiled by crbug.com/559117.
The race happens only when a ConvertableToTraceFormat is passed to
AddMetadataEvent. Explanation of the race:
 - AddMetadataEvent takes optional ConvertableToTraceFormat args.
 - ConvertableToTraceFormat is RefCounted but not RCThreadSafe.
 - AddMetadataEvent acquires the global TraceLog lock_ (which is good)
   but does that only after having initialized the trace_event.
 - CTTF args are copied into the TraceEvent outside of the TraceLog
   lock. The underlying CTTF AddRef/Deref happen outside the lock.
 - Later, when we get to AddMetadataEventsWhileLocked we try to
   re-AddRef the CTTF args.
At this point TSAN barks because the CTTF refcount is written both
ouside and inside the lock.
The fix is trivial, just anticipate the lock of AddMetadataEvent by
one statement.

BUG=559117
TBR=oysteine@chromium.org
==========

[petrcermak, 2016-01-14 11:58:17]

petrcermak@chromium.org changed reviewers:
+ petrcermak@chromium.org

[petrcermak, 2016-01-14 11:58:18]

LGTM with zero comments.

Thanks,
Petr

[Primiano Tucci (use gerrit), 2016-01-14 11:58:45]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2016-01-14 11:58:57]

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

[commit-bot: I haz the power, 2016-01-14 12:33:54]

Description was changed from

==========
tracing: Fix AddMetadataEvent race in presence of Convertable arguments

crrev.com/1421053002 introduced a TraceLog::AddMetadataEvent method.
This method has a little race, unveiled by crbug.com/559117.
The race happens only when a ConvertableToTraceFormat is passed to
AddMetadataEvent. Explanation of the race:
 - AddMetadataEvent takes optional ConvertableToTraceFormat args.
 - ConvertableToTraceFormat is RefCounted but not RCThreadSafe.
 - AddMetadataEvent acquires the global TraceLog lock_ (which is good)
   but does that only after having initialized the trace_event.
 - CTTF args are copied into the TraceEvent outside of the TraceLog
   lock. The underlying CTTF AddRef/Deref happen outside the lock.
 - Later, when we get to AddMetadataEventsWhileLocked we try to
   re-AddRef the CTTF args.
At this point TSAN barks because the CTTF refcount is written both
ouside and inside the lock.
The fix is trivial, just anticipate the lock of AddMetadataEvent by
one statement.

BUG=559117
TBR=oysteine@chromium.org
==========

to

==========
tracing: Fix AddMetadataEvent race in presence of Convertable arguments

crrev.com/1421053002 introduced a TraceLog::AddMetadataEvent method.
This method has a little race, unveiled by crbug.com/559117.
The race happens only when a ConvertableToTraceFormat is passed to
AddMetadataEvent. Explanation of the race:
 - AddMetadataEvent takes optional ConvertableToTraceFormat args.
 - ConvertableToTraceFormat is RefCounted but not RCThreadSafe.
 - AddMetadataEvent acquires the global TraceLog lock_ (which is good)
   but does that only after having initialized the trace_event.
 - CTTF args are copied into the TraceEvent outside of the TraceLog
   lock. The underlying CTTF AddRef/Deref happen outside the lock.
 - Later, when we get to AddMetadataEventsWhileLocked we try to
   re-AddRef the CTTF args.
At this point TSAN barks because the CTTF refcount is written both
ouside and inside the lock.
The fix is trivial, just anticipate the lock of AddMetadataEvent by
one statement.

BUG=559117
TBR=oysteine@chromium.org
==========

[commit-bot: I haz the power, 2016-01-14 12:33:55]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-01-14 12:35:53]

Description was changed from

==========
tracing: Fix AddMetadataEvent race in presence of Convertable arguments

crrev.com/1421053002 introduced a TraceLog::AddMetadataEvent method.
This method has a little race, unveiled by crbug.com/559117.
The race happens only when a ConvertableToTraceFormat is passed to
AddMetadataEvent. Explanation of the race:
 - AddMetadataEvent takes optional ConvertableToTraceFormat args.
 - ConvertableToTraceFormat is RefCounted but not RCThreadSafe.
 - AddMetadataEvent acquires the global TraceLog lock_ (which is good)
   but does that only after having initialized the trace_event.
 - CTTF args are copied into the TraceEvent outside of the TraceLog
   lock. The underlying CTTF AddRef/Deref happen outside the lock.
 - Later, when we get to AddMetadataEventsWhileLocked we try to
   re-AddRef the CTTF args.
At this point TSAN barks because the CTTF refcount is written both
ouside and inside the lock.
The fix is trivial, just anticipate the lock of AddMetadataEvent by
one statement.

BUG=559117
TBR=oysteine@chromium.org
==========

to

==========
tracing: Fix AddMetadataEvent race in presence of Convertable arguments

crrev.com/1421053002 introduced a TraceLog::AddMetadataEvent method.
This method has a little race, unveiled by crbug.com/559117.
The race happens only when a ConvertableToTraceFormat is passed to
AddMetadataEvent. Explanation of the race:
 - AddMetadataEvent takes optional ConvertableToTraceFormat args.
 - ConvertableToTraceFormat is RefCounted but not RCThreadSafe.
 - AddMetadataEvent acquires the global TraceLog lock_ (which is good)
   but does that only after having initialized the trace_event.
 - CTTF args are copied into the TraceEvent outside of the TraceLog
   lock. The underlying CTTF AddRef/Deref happen outside the lock.
 - Later, when we get to AddMetadataEventsWhileLocked we try to
   re-AddRef the CTTF args.
At this point TSAN barks because the CTTF refcount is written both
ouside and inside the lock.
The fix is trivial, just anticipate the lock of AddMetadataEvent by
one statement.

BUG=559117
TBR=oysteine@chromium.org

Committed: https://crrev.com/5c0f93b3235b44f0c69b23063fa10e49da067666
Cr-Commit-Position: refs/heads/master@{#369401}
==========

[commit-bot: I haz the power, 2016-01-14 12:35:54]

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