Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix concurrent sparkplug #47461

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.3',
'v8_embedder_string': '-node.5',

##### V8 defaults for Node.js #####

Expand Down
24 changes: 13 additions & 11 deletions deps/v8/src/heap/collection-barrier.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@
namespace v8 {
namespace internal {

CollectionBarrier::CollectionBarrier(Heap* heap)
: heap_(heap),
foreground_task_runner_(V8::GetCurrentPlatform()->GetForegroundTaskRunner(
reinterpret_cast<v8::Isolate*>(heap->isolate()))) {}

bool CollectionBarrier::WasGCRequested() {
return collection_requested_.load();
}
Expand Down Expand Up @@ -95,7 +100,14 @@ bool CollectionBarrier::AwaitCollectionBackground(LocalHeap* local_heap) {
}

// The first thread needs to activate the stack guard and post the task.
if (first_thread) ActivateStackGuardAndPostTask();
if (first_thread) {
Isolate* isolate = heap_->isolate();
ExecutionAccess access(isolate);
isolate->stack_guard()->RequestGC();

foreground_task_runner_->PostTask(
std::make_unique<BackgroundCollectionInterruptTask>(heap_));
}

ParkedScope scope(local_heap);
base::MutexGuard guard(&mutex_);
Expand All @@ -109,16 +121,6 @@ bool CollectionBarrier::AwaitCollectionBackground(LocalHeap* local_heap) {
return collection_performed_;
}

void CollectionBarrier::ActivateStackGuardAndPostTask() {
Isolate* isolate = heap_->isolate();
ExecutionAccess access(isolate);
isolate->stack_guard()->RequestGC();

V8::GetCurrentPlatform()
->GetForegroundTaskRunner(reinterpret_cast<v8::Isolate*>(isolate))
->PostTask(std::make_unique<BackgroundCollectionInterruptTask>(heap_));
}

void CollectionBarrier::StopTimeToCollectionTimer() {
if (collection_requested_.load()) {
base::MutexGuard guard(&mutex_);
Expand Down
8 changes: 4 additions & 4 deletions deps/v8/src/heap/collection-barrier.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class Heap;
// This class stops and resumes all background threads waiting for GC.
class CollectionBarrier {
public:
explicit CollectionBarrier(Heap* heap) : heap_(heap) {}
explicit CollectionBarrier(Heap* heap);

// Returns true when collection was requested.
bool WasGCRequested();
Expand All @@ -49,9 +49,6 @@ class CollectionBarrier {
bool AwaitCollectionBackground(LocalHeap* local_heap);

private:
// Activate stack guards and posting a task to perform the GC.
void ActivateStackGuardAndPostTask();

Heap* heap_;
base::Mutex mutex_;
base::ConditionVariable cv_wakeup_;
Expand All @@ -72,6 +69,9 @@ class CollectionBarrier {

// Will be set as soon as Isolate starts tear down.
bool shutdown_requested_ = false;

// Used to post tasks on the main thread.
std::shared_ptr<v8::TaskRunner> foreground_task_runner_;
};

} // namespace internal
Expand Down
3 changes: 2 additions & 1 deletion deps/v8/src/heap/heap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,6 @@ Heap::Heap()
allocation_type_for_in_place_internalizable_strings_(
isolate()->OwnsStringTables() ? AllocationType::kOld
: AllocationType::kSharedOld),
collection_barrier_(new CollectionBarrier(this)),
marking_state_(isolate_),
non_atomic_marking_state_(isolate_),
atomic_marking_state_(isolate_),
Expand Down Expand Up @@ -5431,6 +5430,8 @@ void Heap::SetUp(LocalHeap* main_thread_local_heap) {
code_page_allocator = isolate_->page_allocator();
}

collection_barrier_.reset(new CollectionBarrier(this));

// Set up memory allocator.
memory_allocator_.reset(
new MemoryAllocator(isolate_, code_page_allocator, MaxReserved()));
Expand Down
16 changes: 9 additions & 7 deletions deps/v8/src/heap/incremental-marking-job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ class IncrementalMarkingJob::Task : public CancelableTask {
const StackState stack_state_;
};

IncrementalMarkingJob::IncrementalMarkingJob(Heap* heap) V8_NOEXCEPT
: heap_(heap),
foreground_task_runner_(V8::GetCurrentPlatform()->GetForegroundTaskRunner(
reinterpret_cast<v8::Isolate*>(heap->isolate()))) {}

void IncrementalMarkingJob::ScheduleTask() {
base::MutexGuard guard(&mutex_);

Expand All @@ -44,22 +49,19 @@ void IncrementalMarkingJob::ScheduleTask() {
return;
}

v8::Isolate* isolate = reinterpret_cast<v8::Isolate*>(heap_->isolate());
is_task_pending_ = true;
auto taskrunner = V8::GetCurrentPlatform()->GetForegroundTaskRunner(isolate);

const auto stack_state = taskrunner->NonNestableTasksEnabled()
const auto stack_state = foreground_task_runner_->NonNestableTasksEnabled()
? StackState::kNoHeapPointers
: StackState::kMayContainHeapPointers;

auto task = std::make_unique<Task>(heap_->isolate(), this, stack_state);

scheduled_time_ = heap_->MonotonicallyIncreasingTimeInMs();

if (taskrunner->NonNestableTasksEnabled()) {
taskrunner->PostNonNestableTask(std::move(task));
if (foreground_task_runner_->NonNestableTasksEnabled()) {
foreground_task_runner_->PostNonNestableTask(std::move(task));
} else {
taskrunner->PostTask(std::move(task));
foreground_task_runner_->PostTask(std::move(task));
}
}

Expand Down
3 changes: 2 additions & 1 deletion deps/v8/src/heap/incremental-marking-job.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class Isolate;
// step and posts another task until the marking is completed.
class IncrementalMarkingJob final {
public:
explicit IncrementalMarkingJob(Heap* heap) V8_NOEXCEPT : heap_(heap) {}
explicit IncrementalMarkingJob(Heap* heap) V8_NOEXCEPT;

void ScheduleTask();
double CurrentTimeToTask() const;
Expand All @@ -31,6 +31,7 @@ class IncrementalMarkingJob final {
base::Mutex mutex_;
double scheduled_time_ = 0.0;
bool is_task_pending_ = false;
std::shared_ptr<v8::TaskRunner> foreground_task_runner_;
};
} // namespace internal
} // namespace v8
Expand Down
8 changes: 4 additions & 4 deletions src/node_platform.cc
Original file line number Diff line number Diff line change
Expand Up @@ -451,10 +451,10 @@ void NodePlatform::DrainTasks(Isolate* isolate) {
std::shared_ptr<PerIsolatePlatformData> per_isolate = ForNodeIsolate(isolate);
if (!per_isolate) return;

do {
// Worker tasks aren't associated with an Isolate.
worker_thread_task_runner_->BlockingDrain();
} while (per_isolate->FlushForegroundTasksInternal());
// Drain foreground tasks but not worker tasks as this may cause deadlocks
// and v8::Isolate::Dispose will join V8's worker tasks for that isolate.
while (per_isolate->FlushForegroundTasksInternal()) {
}
}

bool PerIsolatePlatformData::FlushForegroundTasksInternal() {
Expand Down
2 changes: 0 additions & 2 deletions test/parallel/parallel.status
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ test-http-server-request-timeouts-mixed: PASS,FLAKY
# https://github.com/nodejs/node/pull/31178
test-crypto-dh-stateless: SKIP
test-crypto-keygen: SKIP
# https://github.com/nodejs/node/issues/47297
test-wasm-web-api: SKIP

[$system==solaris] # Also applies to SmartOS
# https://github.com/nodejs/node/issues/43457
Expand Down
3 changes: 0 additions & 3 deletions test/wpt/status/wasm/webapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,5 @@
},
"status.any.js": {
"skip": "WPTRunner does not support fetch()"
},
"instantiateStreaming-bad-imports.any.js": {
"skip": "Flaky on ARM with V8 >= 11.2"
}
}