• Peter Marshall's avatar
    Revert "[cpu-profiler] Ensure sampled thread has Isolate lock under Windows" · 32435062
    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: 's avatarPeter Marshall <petermarshall@chromium.org>
    Commit-Queue: Peter Marshall <petermarshall@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#69638}
    32435062
Name
Last commit
Last update
..
compiler Loading commit data...
heap Loading commit data...
interpreter Loading commit data...
libplatform Loading commit data...
libsampler Loading commit data...
parsing Loading commit data...
torque Loading commit data...
wasm Loading commit data...
BUILD.gn Loading commit data...
DEPS Loading commit data...
OWNERS Loading commit data...
assembler-helper-arm.cc Loading commit data...
assembler-helper-arm.h Loading commit data...
cctest.cc Loading commit data...
cctest.h Loading commit data...
cctest.status Loading commit data...
collector.h Loading commit data...
disasm-regex-helper.cc Loading commit data...
disasm-regex-helper.h Loading commit data...
expression-type-collector-macros.h Loading commit data...
gay-fixed.cc Loading commit data...
gay-fixed.h Loading commit data...
gay-precision.cc Loading commit data...
gay-precision.h Loading commit data...
gay-shortest.cc Loading commit data...
gay-shortest.h Loading commit data...
manually-externalized-buffer.h Loading commit data...
print-extension.cc Loading commit data...
print-extension.h Loading commit data...
profiler-extension.cc Loading commit data...
profiler-extension.h Loading commit data...
scope-test-helper.h Loading commit data...
setup-isolate-for-tests.cc Loading commit data...
setup-isolate-for-tests.h Loading commit data...
test-access-checks.cc Loading commit data...
test-accessor-assembler.cc Loading commit data...
test-accessors.cc Loading commit data...
test-allocation.cc Loading commit data...
test-api-accessors.cc Loading commit data...
test-api-array-buffer.cc Loading commit data...
test-api-icu.cc Loading commit data...
test-api-interceptors.cc Loading commit data...
test-api-stack-traces.cc Loading commit data...
test-api-typed-array.cc Loading commit data...
test-api-wasm.cc Loading commit data...
test-api.cc Loading commit data...
test-api.h Loading commit data...
test-array-list.cc Loading commit data...
test-assembler-arm.cc Loading commit data...
test-assembler-arm64.cc Loading commit data...
test-assembler-ia32.cc Loading commit data...
test-assembler-mips.cc Loading commit data...
test-assembler-mips64.cc Loading commit data...
test-assembler-ppc.cc Loading commit data...
test-assembler-s390.cc Loading commit data...
test-assembler-x64.cc Loading commit data...
test-atomicops.cc Loading commit data...
test-backing-store.cc Loading commit data...
test-bignum-dtoa.cc Loading commit data...
test-bignum.cc Loading commit data...
test-bit-vector.cc Loading commit data...
test-circular-queue.cc Loading commit data...
test-code-layout.cc Loading commit data...
test-code-pages.cc Loading commit data...
test-code-stub-assembler.cc Loading commit data...
test-compiler.cc Loading commit data...
test-concurrent-descriptor-array.cc Loading commit data...
test-concurrent-prototype.cc Loading commit data...
test-concurrent-script-context-table.cc Loading commit data...
test-concurrent-transition-array.cc Loading commit data...
test-constantpool.cc Loading commit data...
test-conversions.cc Loading commit data...
test-cpu-profiler.cc Loading commit data...
test-date.cc Loading commit data...
test-debug-helper.cc Loading commit data...
test-debug.cc Loading commit data...
test-decls.cc Loading commit data...
test-deoptimization.cc Loading commit data...
test-dictionary.cc Loading commit data...
test-disasm-arm.cc Loading commit data...
test-disasm-arm64.cc Loading commit data...
test-disasm-ia32.cc Loading commit data...
test-disasm-mips.cc Loading commit data...
test-disasm-mips64.cc Loading commit data...
test-disasm-ppc.cc Loading commit data...
test-disasm-s390.cc Loading commit data...
test-disasm-x64.cc Loading commit data...
test-diy-fp.cc Loading commit data...
test-double.cc Loading commit data...
test-dtoa.cc Loading commit data...
test-elements-kind.cc Loading commit data...
test-factory.cc Loading commit data...
test-fast-dtoa.cc Loading commit data...
test-feedback-vector.cc Loading commit data...
test-feedback-vector.h Loading commit data...
test-field-type-tracking.cc Loading commit data...
test-fixed-dtoa.cc Loading commit data...
test-flags.cc Loading commit data...
test-func-name-inference.cc Loading commit data...
test-fuzz-arm64.cc Loading commit data...
test-global-handles.cc Loading commit data...
test-global-object.cc Loading commit data...
test-hashcode.cc Loading commit data...
test-hashmap.cc Loading commit data...
test-heap-profiler.cc Loading commit data...
test-icache.cc Loading commit data...
test-identity-map.cc Loading commit data...
test-inobject-slack-tracking.cc Loading commit data...
test-inspector.cc Loading commit data...
test-intl.cc Loading commit data...
test-javascript-arm64.cc Loading commit data...
test-js-arm64-variables.cc Loading commit data...
test-js-weak-refs.cc Loading commit data...
test-liveedit.cc Loading commit data...
test-local-handles.cc Loading commit data...
test-lockers.cc Loading commit data...
test-log-stack-tracer.cc Loading commit data...
test-log.cc Loading commit data...
test-macro-assembler-arm.cc Loading commit data...
test-macro-assembler-arm64.cc Loading commit data...
test-macro-assembler-mips.cc Loading commit data...
test-macro-assembler-mips64.cc Loading commit data...
test-macro-assembler-x64.cc Loading commit data...
test-managed.cc Loading commit data...
test-mementos.cc Loading commit data...
test-modules.cc Loading commit data...
test-object.cc Loading commit data...
test-orderedhashtable.cc Loading commit data...
test-parsing.cc Loading commit data...
test-persistent-handles.cc Loading commit data...
test-platform.cc Loading commit data...
test-pointer-auth-arm64.cc Loading commit data...
test-poison-disasm-arm.cc Loading commit data...
test-poison-disasm-arm64.cc Loading commit data...
test-profile-generator.cc Loading commit data...
test-random-number-generator.cc Loading commit data...
test-regexp.cc Loading commit data...
test-representation.cc Loading commit data...
test-roots.cc Loading commit data...
test-sampler-api.cc Loading commit data...
test-serialize.cc Loading commit data...
test-smi-lexicographic-compare.cc Loading commit data...
test-stack-unwinding-win64.cc Loading commit data...
test-strings.cc Loading commit data...
test-strtod.cc Loading commit data...
test-symbols.cc Loading commit data...
test-sync-primitives-arm.cc Loading commit data...
test-sync-primitives-arm64.cc Loading commit data...
test-thread-termination.cc Loading commit data...
test-threads.cc Loading commit data...
test-trace-event.cc Loading commit data...
test-traced-value.cc Loading commit data...
test-transitions.cc Loading commit data...
test-transitions.h Loading commit data...
test-typedarrays.cc Loading commit data...
test-types.cc Loading commit data...
test-unboxed-doubles.cc Loading commit data...
test-unscopables-hidden-prototype.cc Loading commit data...
test-unwinder-code-pages.cc Loading commit data...
test-usecounters.cc Loading commit data...
test-utils-arm64.cc Loading commit data...
test-utils-arm64.h Loading commit data...
test-utils.cc Loading commit data...
test-v8windbg.cc Loading commit data...
test-version.cc Loading commit data...
test-weakmaps.cc Loading commit data...
test-weaksets.cc Loading commit data...
testcfg.py Loading commit data...
trace-extension.cc Loading commit data...
trace-extension.h Loading commit data...
unicode-helpers.cc Loading commit data...
unicode-helpers.h Loading commit data...