-
Peter Marshall authored
This reverts commit dfb3f7da. Reason for revert: Breaks LSAN & ASAN flakily: https://bugs.chromium.org/p/v8/issues/detail?id=10861 Original change's description: > [cpu-profiler] Ensure sampled thread has Isolate lock under Windows > > While the sampler checked if the sampled thread had the Isolate locked > (if locks are being used) under Linux, the check was not done under > Windows (or Fuchsia) which meant that in a multi-threading application > under Windows, thread locking was not checked making it prone to seg > faults and the like as the profiler would be extracting info from a > heap in motion. The fix was to move the lock check into CpuSampler > and Ticker (--prof) so all OSes would do the correct check. > > The basic concept is that on all operating systems a CpuProfiler, and > so its corresponding CpuCampler, the profiler is tied to a thread. > This is not based on first principles or anything, it's simply the > way it works in V8, though it is a useful conceit as it makes > visualization and interpretation of profile data much easier. > > To collect a sample on a thread associated with a profiler the thread > must be stopped for obvious reasons -- walking the stack of a running > thread is a formula for disaster. The mechanism for stopping a thread > is OS-specific and is done in sample.cc. There are currently three > basic approaches, one for Linux/Unix variants, one for Windows and one > for Fuchsia. The approaches vary as to which thread actually collects > the sample -- under Linux the sample is actually collected on the > (interrupted) sampled thread whereas under Fuchsia/Windows it's on > a separate thread. > > However, in a multi-threaded environment (where Locker is used), it's > not sufficient for the sampled thread to be stopped. Because the stack > walk involves looking in the Isolate heap, no other thread can be > messing with the heap while the sample is collected. The only ways to > ensure this would be to either stop all threads whenever collecting a > sample, or to ensure that the thread being sampled holds the Isolate > lock so prevents other threads from messing with the heap. While there > might be something to be said for the "stop all threads" approach, the > current approach in V8 is to only stop the sampled thread so, if in a > multi-threaded environment, the profiler must check if the thread being > sampled holds the Isolate lock. > > Since this check must be done, independent of which thread the sample > is being collected on (since it varies from OS to OS), the approach is > to save the thread id of the thread to be profiled/sampled when the > CpuSampler is instantiated (on all OSes it is instantiated on the > sampled thread) and then check that thread id against the Isolate lock > holder thread id before collecting a sample. If it matches, we know > sample.cc has stop the sampled thread, one way or another, and we know > that no other thread can mess with the heap (since the stopped thread > holds the Isolate lock) so it's safe to walk the stack and collect data > from the heap so the sample can be taken. It it doesn't match, we can't > safely collect the sample so we don't. > > Bug: v8:10850 > Change-Id: Iab2493130b9328430d7e5f5d3cf90ad6d10b1892 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2377108 > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Commit-Queue: Peter Marshall <petermarshall@chromium.org> > Cr-Commit-Position: refs/heads/master@{#69623} TBR=akodat@rocketsoftware.com,petermarshall@chromium.org,petermarshall@google.com Change-Id: Ib6b6dc4ce109d5aa4e504fa7c9769f5cd95ddd0c No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: v8:10850 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2387570Reviewed-by: Peter Marshall <petermarshall@chromium.org> Commit-Queue: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#69638}
32435062