• Alex Kodat's avatar
    [cpu-profiler] Ensure sampled thread has Isolate lock under Windows · 76217f57
    Alex Kodat authored
    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 using isolate->js_entry_sp
    to determine the stack to walk but isolate->js_entry_sp is the stack
    pointer for the thread that currently has the Isolate lock so, if the
    sampled thread does not have the lock, the sampler woud be iterating
    over the wrong stack, one that might actually be actively changing on
    another thread. 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: Iba6cabcd3e11a19c261c004103e37e806934dc6f
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2411343Reviewed-by: 's avatarPeter Marshall <petermarshall@chromium.org>
    Commit-Queue: Peter Marshall <petermarshall@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#69952}
    76217f57
Name
Last commit
Last update
build_overrides Loading commit data...
custom_deps Loading commit data...
docs Loading commit data...
gni Loading commit data...
include Loading commit data...
infra Loading commit data...
samples Loading commit data...
src Loading commit data...
test Loading commit data...
testing Loading commit data...
third_party Loading commit data...
tools Loading commit data...
.clang-format Loading commit data...
.clang-tidy Loading commit data...
.editorconfig Loading commit data...
.flake8 Loading commit data...
.git-blame-ignore-revs Loading commit data...
.gitattributes Loading commit data...
.gitignore Loading commit data...
.gn Loading commit data...
.vpython Loading commit data...
.ycm_extra_conf.py Loading commit data...
AUTHORS Loading commit data...
BUILD.gn Loading commit data...
CODE_OF_CONDUCT.md Loading commit data...
COMMON_OWNERS Loading commit data...
DEPS Loading commit data...
ENG_REVIEW_OWNERS Loading commit data...
INFRA_OWNERS Loading commit data...
INTL_OWNERS Loading commit data...
LICENSE Loading commit data...
LICENSE.fdlibm Loading commit data...
LICENSE.strongtalk Loading commit data...
LICENSE.v8 Loading commit data...
MIPS_OWNERS Loading commit data...
OWNERS Loading commit data...
PPC_OWNERS Loading commit data...
PRESUBMIT.py Loading commit data...
README.md Loading commit data...
S390_OWNERS Loading commit data...
WATCHLISTS Loading commit data...
codereview.settings Loading commit data...