[vm] More TSAN profiler issues.
TEST=tsan
Bug: https://github.com/dart-lang/sdk/issues/61594
Change-Id: I82598218a98b764d66deb671f8ec841c8e1c3cab
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/452280
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h
index b608748..056de9b 100644
--- a/runtime/vm/isolate.h
+++ b/runtime/vm/isolate.h
@@ -1195,6 +1195,9 @@
void set_current_sample_block(SampleBlock* block) {
current_sample_block_ = block;
}
+ SampleBlock* exchange_current_sample_block(SampleBlock* block) {
+ return current_sample_block_.exchange(block, std::memory_order_acq_rel);
+ }
void ProcessFreeSampleBlocks(Thread* thread);
// Returns the current SampleBlock used to track Dart allocation samples.
@@ -1204,6 +1207,10 @@
void set_current_allocation_sample_block(SampleBlock* block) {
current_allocation_sample_block_ = block;
}
+ SampleBlock* exchange_current_allocation_sample_block(SampleBlock* block) {
+ return current_allocation_sample_block_.exchange(block,
+ std::memory_order_acq_rel);
+ }
bool TakeHasCompletedBlocks() {
return has_completed_blocks_.exchange(0) != 0;
diff --git a/runtime/vm/profiler.cc b/runtime/vm/profiler.cc
index 1b43c09..e73077f 100644
--- a/runtime/vm/profiler.cc
+++ b/runtime/vm/profiler.cc
@@ -794,15 +794,14 @@
static void FlushSampleBlocks(Isolate* isolate) {
ASSERT(isolate != nullptr);
- SampleBlock* block = isolate->current_sample_block();
+ SampleBlock* block = isolate->exchange_current_sample_block(nullptr);
if (block != nullptr) {
- isolate->set_current_sample_block(nullptr);
block->MarkCompleted();
}
- block = isolate->current_allocation_sample_block();
+ block = isolate->exchange_current_allocation_sample_block(nullptr);
if (block != nullptr) {
- isolate->set_current_allocation_sample_block(nullptr);
+ // Allocation samples are collected synchronously.
block->MarkCompleted();
}
}
@@ -1416,6 +1415,25 @@
sample->SetAt(0, pc);
}
+void ReleaseToCurrentBlock(Isolate* isolate) {
+#if defined(DART_HOST_OS_MACOS) || defined(DART_HOST_OS_WINDOWS) || \
+ defined(DART_HOST_OS_FUCHSIA)
+ // The sample is collected by a different thread. The sample appears all at
+ // once from the profiled thread's point of view. Establish the isolate
+ // flushing its own current block happens-after the most recent sample
+ // written in that block by dumping a dependency through the current block.
+ // TSAN doesn't otherwise know this is already true because it doesn't have
+ // special treatment for thread_suspend/resume.
+ SampleBlock* block = isolate->current_sample_block();
+ isolate->exchange_current_sample_block(block);
+#elif defined(DART_HOST_OS_LINUX) || defined(DART_HOST_OS_ANDROID)
+ // The sample is collected by a signal handler on the same thread being
+ // sampled.
+#else
+#error What kind of sampler?
+#endif
+}
+
void Profiler::SampleThread(Thread* thread,
const InterruptedThreadState& state) {
ASSERT(thread != nullptr);
@@ -1494,6 +1512,7 @@
if (thread->IGNORE_RACE2(IsDeoptimizing)()) {
counters_.single_frame_sample_deoptimizing.fetch_add(1);
SampleThreadSingleFrame(thread, sample, pc);
+ ReleaseToCurrentBlock(isolate);
return;
}
}
@@ -1505,6 +1524,7 @@
counters_.single_frame_sample_get_and_validate_stack_bounds.fetch_add(1);
// Could not get stack boundary.
SampleThreadSingleFrame(thread, sample, pc);
+ ReleaseToCurrentBlock(isolate);
return;
}
@@ -1531,6 +1551,7 @@
CollectSample(isolate, exited_dart_code, in_dart_code, sample,
&native_stack_walker, &dart_stack_walker, pc, fp, sp,
&counters_);
+ ReleaseToCurrentBlock(isolate);
}
CodeDescriptor::CodeDescriptor(const AbstractCode code) : code_(code) {}
diff --git a/runtime/vm/profiler_test.cc b/runtime/vm/profiler_test.cc
index e72278b..de90bd6 100644
--- a/runtime/vm/profiler_test.cc
+++ b/runtime/vm/profiler_test.cc
@@ -4,6 +4,7 @@
#include "platform/assert.h"
+#include "platform/thread_sanitizer.h"
#include "vm/dart_api_impl.h"
#include "vm/dart_api_state.h"
#include "vm/flags.h"
@@ -29,19 +30,27 @@
// SampleVisitor ignores samples with pc == 0.
const uword kValidPc = 0xFF;
+NO_SANITIZE_THREAD
+static void SetIgnoreRace(bool* flag, bool value) {
+ *flag = value;
+}
+
// Some tests are written assuming native stack trace profiling is disabled.
class DisableNativeProfileScope : public ValueObject {
public:
DisableNativeProfileScope()
: FLAG_profile_vm_(FLAG_profile_vm),
FLAG_profile_vm_allocation_(FLAG_profile_vm_allocation) {
- FLAG_profile_vm = false;
- FLAG_profile_vm_allocation = false;
+ // Ignore race: we generally don't try to ensure the profiler is disabled
+ // when changing setting for unit tests. On Linux/Android, there is no way
+ // to wait for outstanding SIGPROFs.
+ SetIgnoreRace(&FLAG_profile_vm, false);
+ SetIgnoreRace(&FLAG_profile_vm_allocation, false);
}
~DisableNativeProfileScope() {
- FLAG_profile_vm = FLAG_profile_vm_;
- FLAG_profile_vm_allocation = FLAG_profile_vm_allocation_;
+ SetIgnoreRace(&FLAG_profile_vm, FLAG_profile_vm_);
+ SetIgnoreRace(&FLAG_profile_vm_allocation, FLAG_profile_vm_allocation_);
}
private:
