commit a6a99dc45fc250fe9f6429d7fd4df344c37691fe
author Vyacheslav Egorov <vegorov@google.com> Wed May 07 07:18:41 2025 -0700
committer Commit Queue <dart-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed May 07 07:18:41 2025 -0700
tree 4aba494b0d20c89466a242f219c6a96253eb12cf
parent 2455ace91bad15da066ad7aee164f65952eb18cd

[vm] Fix issues in SampleBlockProcessor

This is follow up to commit cb0c2bf5edff91e0242ef14d4b52db7a02637c7d.

SampleBlockProcessor only enters isolate group and not a specific
isolate so the code must not rely on thread->isolate(). This fixes two
places where this was not the case:

* ProfileBuilder::IsPCInDartHeap
* UserTags::TagName

pkg/dds/test/get_cached_cpu_samples_test was supposed to cover this but
it has two problems:

First I observed that SampleBlockProcessor never gets a chance to
process a block if mutator thread always gets to it first (via a
scheduled interrupt), so this code is not well exercised. I started by
adding a variant of the test where interrupts are inhibited via a
vm:unsafe:no-interrupts pragma - which revealed the crashes in the
SampleBlockProcessor code.

This revealed the second problem: get_cached_cpu_samples_test does not
actually fail if testee crashes during the test, it just silently
completes with success. This seems to happen because disposal of
VmService connection is not forwarded into the future on which the test
is awaiting - and the whole process just exits once VmService connection
to the testee disappears (because all ports are closed, no pending
activity is possible after that one). I have fixed this by adding a
helper function which checks that connection to VmService only goes away
when we dispose it.

Note: there is another obvious issue here, which I am leaving unfixed
for now. SampleBlockProcessor calls UserTags::TagName in a way that can
race with isolate itself modifying the table. I think this race is
extremely unlikely but it can cause crashes on ARMs with its weak memory
model (e.g. we might end up reading garbage due to the reordering of
stores).

TEST=pkg/dds/test/get_cached_cpu_samples_test

Change-Id: Iee15ec2b019928b798c312e63edc76696abf5527
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426300
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>