Tracing tests failing on multiple builders |
|||
Issue descriptionFiled by sheriff-o-matic@appspot.gserviceaccount.com on behalf of dmazzoni@chromium.org services_unittests MemoryTracingIntegrationTest.GenerationChangeDoesntReenterMDM content_browsertests MemoryTracingTest.BrowserInitiatedDump Builders failed on: - linux-chromeos-dbg: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-chromeos-dbg - Mac10.13 Tests (dbg): https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.13%20Tests%20%28dbg%29 Suspecting: https://chromium-review.googlesource.com/c/chromium/src/+/1298997
,
Oct 31
,
Nov 2
The revert appears to have resolved the test failures. Removing from sheriff queue.
,
Nov 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b01ce1745dfcde22bbb4290be99030495862a1a9 commit b01ce1745dfcde22bbb4290be99030495862a1a9 Author: David 'Digit' Turner <digit@google.com> Date: Mon Nov 19 08:37:22 2018 Avoid dangling pointers in TraceEvent::Reset(). This patch fixed the base::trace_event::TraceEvent::Reset() method to ensure that it never creates dangling pointers. This can happen in the following case: - TraceEvent::Initialize() is called on an instance, with TRACE_EVENT_FLAG_COPY set in the |flags| argument. This will copy argument names, copyable string values, as well as the name and scope into a single heap allocated buffer backed by |parameter_copy_storage_|, and will also adjust all internal pointer fields to point to it. - TraceEvent::Reset() is called on the same instance, this frees the storage area, but before this CL did not update the internal pointers, who were now dangling into heap-free memory! - Later, some code will iterate over the arguments with a loop like: for (int i = 0; i < kTraceMaxNumArgs && arg_names_[i] != nullptr; ++i) { ... } The assumption being that an arg_names_[i] value of nullptr indicates the end of list. Unfortunately, in the case above, this will read completely invalid values from memory. + Fix TraceEvent::MoveFrom() to call other->Reset() to ensure that the source instance is left in consistent state. I believe this is the source of flakiness on many tests related to TraceEvent, and hope this fixes it. BUG=905624,899813 R=oystene@chromium.org,primiano@chromium.org,alexilin@chromium.org,pkl@chromium.org Change-Id: I63cbadc728130cddc68b8c92b28e1e3f584793f4 Reviewed-on: https://chromium-review.googlesource.com/c/1340308 Commit-Queue: David Turner <digit@chromium.org> Reviewed-by: Peter Lee <pkl@chromium.org> Reviewed-by: oysteine <oysteine@chromium.org> Cr-Commit-Position: refs/heads/master@{#609207} [modify] https://crrev.com/b01ce1745dfcde22bbb4290be99030495862a1a9/base/trace_event/trace_event_impl.cc |
|||
►
Sign in to add a comment |
|||
Comment 1 by bugdroid1@chromium.org
, Oct 29