Commit 6114d098 authored by Clemens Backes's avatar Clemens Backes Committed by V8 LUCI CQ

[d8] Make counters fully atomic

Counter updates were already atomic, but reading the counter values was
not. This lead to data races if one isolate called `quit` while other
isolates were still running.
This makes counters fully atomic, and reflects that by making the fields
{std::atomic<int>}.

R=mlippautz@chromium.org

Bug: v8:12481, v8:12482
Change-Id: I6fc78ad6461b93c4b3e87bec052b0a67694539e3
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
Cq-Include-Trybots: luci.v8.try:v8_linux64_ubsan_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3320428Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78279}
parent f0c982b8
......@@ -2721,19 +2721,16 @@ void Shell::ReportException(v8::Isolate* isolate, v8::TryCatch* try_catch) {
ReportException(isolate, try_catch->Message(), try_catch->Exception());
}
int32_t* Counter::Bind(const char* name, bool is_histogram) {
int i;
for (i = 0; i < kMaxNameSize - 1 && name[i]; i++)
name_[i] = static_cast<char>(name[i]);
name_[i] = '\0';
void Counter::Bind(const char* name, bool is_histogram) {
base::OS::StrNCpy(name_, kMaxNameSize, name, kMaxNameSize);
// Explicitly null-terminate, in case {name} is longer than {kMaxNameSize}.
name_[kMaxNameSize - 1] = '\0';
is_histogram_ = is_histogram;
return ptr();
}
void Counter::AddSample(int32_t sample) {
base::Relaxed_AtomicIncrement(reinterpret_cast<base::Atomic32*>(&count_), 1);
base::Relaxed_AtomicIncrement(
reinterpret_cast<base::Atomic32*>(&sample_total_), sample);
void Counter::AddSample(int sample) {
count_.fetch_add(1, std::memory_order_relaxed);
sample_total_.fetch_add(sample, std::memory_order_relaxed);
}
CounterCollection::CounterCollection() {
......
......@@ -45,18 +45,24 @@ struct DynamicImportData;
class Counter {
public:
static const int kMaxNameSize = 64;
int32_t* Bind(const char* name, bool histogram);
int32_t* ptr() { return &count_; }
int32_t count() { return count_; }
int32_t sample_total() { return sample_total_; }
bool is_histogram() { return is_histogram_; }
void Bind(const char* name, bool histogram);
// TODO(12482): Return pointer to an atomic.
int* ptr() {
STATIC_ASSERT(sizeof(int) == sizeof(count_));
return reinterpret_cast<int*>(&count_);
}
int count() const { return count_.load(std::memory_order_relaxed); }
int sample_total() const {
return sample_total_.load(std::memory_order_relaxed);
}
bool is_histogram() const { return is_histogram_; }
void AddSample(int32_t sample);
private:
int32_t count_;
int32_t sample_total_;
std::atomic<int> count_;
std::atomic<int> sample_total_;
bool is_histogram_;
uint8_t name_[kMaxNameSize];
char name_[kMaxNameSize];
};
// A set of counters and associated information. An instance of this
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment