FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

deps: V8: cherry-pick 5f025d1ca2ca · nodejs/node@b16f6da · GitHub

/ node Public

Commit b16f6da

Browse files
targos
committed
deps: V8: cherry-pick 5f025d1ca2ca
Original commit message: [wasm] Fix deadlock in async wrapper compilation If compilation is cancelled while wrapper compilation is running, the tasks spawned for the {AsyncCompileJSToWasmWrapperJob} will return immediately, but {GetMaxConcurrency} will still return a positive value. Hence {Join()} will spawn another task, resulting in a livelock. We could fix this by checking for cancellation in {GetMaxConcurrency}, but that requires taking the compilation state lock. So this CL fixes the issue by dropping the number of outstanding compilation units by to (basically) zero. We can't unconditionally drop to zero because another thread might concurrently execute a wrapper compilation and still call {CompleteUnit} afterwards. Hence only drop outstanding units by the amount of not-yet-started units. R=jkummerow@chromium.org Bug: v8:13858 Change-Id: I5398ef370da2e7f212ca772fd1f87f659929dd6d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4437531 Commit-Queue: Clemens Backes <clemensb@chromium.org> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Cr-Commit-Position: refs/heads/main@{#87143} Refs: v8/v8@5f025d1 PR-URL: #47610 Refs: #47297 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 99f8fca commit b16f6da

2 files changed

Lines changed: 49 additions & 23 deletions

File tree

‎common.gypi‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636

3737
# Reset this number to 0 on major V8 upgrades.
3838
# Increment by one for each non-official patch applied to deps/v8.
39-
'v8_embedder_string': '-node.8',
39+
'v8_embedder_string': '-node.9',
4040

4141
##### V8 defaults for Node.js #####
4242

‎deps/v8/src/wasm/module-compiler.cc‎

Lines changed: 48 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1826,7 +1826,8 @@ void CompileNativeModule(Isolate* isolate,
18261826
class BaseCompileJSToWasmWrapperJob : public JobTask {
18271827
public:
18281828
explicit BaseCompileJSToWasmWrapperJob(size_t compilation_units)
1829-
: outstanding_units_(compilation_units) {}
1829+
: outstanding_units_(compilation_units),
1830+
total_units_(compilation_units) {}
18301831

18311832
size_t GetMaxConcurrency(size_t worker_count) const override {
18321833
size_t flag_limit = static_cast<size_t>(
@@ -1839,12 +1840,20 @@ class BaseCompileJSToWasmWrapperJob : public JobTask {
18391840
}
18401841

18411842
protected:
1842-
// Returns the index of the next unit to process.
1843-
size_t GetNextUnitIndex() {
1844-
// |unit_index_| may exceeed |compilation_units|, but only by the number of
1845-
// workers at worst, thus it can't exceed 2 * |compilation_units| and
1846-
// overflow shouldn't happen.
1847-
return unit_index_.fetch_add(1, std::memory_order_relaxed);
1843+
// Returns {true} and places the index of the next unit to process in
1844+
// {index_out} if there are still units to be processed. Returns {false}
1845+
// otherwise.
1846+
bool GetNextUnitIndex(size_t* index_out) {
1847+
size_t next_index = unit_index_.fetch_add(1, std::memory_order_relaxed);
1848+
if (next_index >= total_units_) {
1849+
// {unit_index_} may exceeed {total_units_}, but only by the number of
1850+
// workers at worst, thus it can't exceed 2 * {total_units_} and overflow
1851+
// shouldn't happen.
1852+
DCHECK_GE(2 * total_units_, next_index);
1853+
return false;
1854+
}
1855+
*index_out = next_index;
1856+
return true;
18481857
}
18491858

18501859
// Returns true if the last unit was completed.
@@ -1855,9 +1864,29 @@ class BaseCompileJSToWasmWrapperJob : public JobTask {
18551864
return outstanding_units == 1;
18561865
}
18571866

1867+
// When external cancellation is detected, call this method to bump
1868+
// {unit_index_} and reset {outstanding_units_} such that no more tasks are
1869+
// being scheduled for this job and all tasks exit as soon as possible.
1870+
void FlushRemainingUnits() {
1871+
// After being cancelled, make sure to reduce outstanding_units_ to
1872+
// *basically* zero, but leave the count positive if other workers are still
1873+
// running, to avoid underflow in {CompleteUnit}.
1874+
size_t next_undone_unit =
1875+
unit_index_.exchange(total_units_, std::memory_order_relaxed);
1876+
size_t undone_units =
1877+
next_undone_unit >= total_units_ ? 0 : total_units_ - next_undone_unit;
1878+
// Note that the caller requested one unit that we also still need to remove
1879+
// from {outstanding_units_}.
1880+
++undone_units;
1881+
size_t previous_outstanding_units =
1882+
outstanding_units_.fetch_sub(undone_units, std::memory_order_relaxed);
1883+
CHECK_LE(undone_units, previous_outstanding_units);
1884+
}
1885+
18581886
private:
18591887
std::atomic<size_t> unit_index_{0};
18601888
std::atomic<size_t> outstanding_units_;
1889+
const size_t total_units_;
18611890
};
18621891

18631892
class AsyncCompileJSToWasmWrapperJob final
@@ -1867,8 +1896,7 @@ class AsyncCompileJSToWasmWrapperJob final
18671896
std::weak_ptr<NativeModule> native_module, size_t compilation_units)
18681897
: BaseCompileJSToWasmWrapperJob(compilation_units),
18691898
native_module_(std::move(native_module)),
1870-
engine_barrier_(GetWasmEngine()->GetBarrierForBackgroundCompile()),
1871-
compilation_units_size_(compilation_units) {}
1899+
engine_barrier_(GetWasmEngine()->GetBarrierForBackgroundCompile()) {}
18721900

18731901
void Run(JobDelegate* delegate) override {
18741902
auto engine_scope = engine_barrier_->TryLock();
@@ -1878,18 +1906,18 @@ class AsyncCompileJSToWasmWrapperJob final
18781906
OperationsBarrier::Token wrapper_compilation_token;
18791907
Isolate* isolate;
18801908

1881-
size_t index = GetNextUnitIndex();
1882-
if (index >= compilation_units_size_) return;
1909+
size_t index;
1910+
if (!GetNextUnitIndex(&index)) return;
18831911
{
18841912
BackgroundCompileScope compile_scope(native_module_);
1885-
if (compile_scope.cancelled()) return;
1913+
if (compile_scope.cancelled()) return FlushRemainingUnits();
18861914
wrapper_unit =
18871915
compile_scope.compilation_state()->GetJSToWasmWrapperCompilationUnit(
18881916
index);
18891917
isolate = wrapper_unit->isolate();
18901918
wrapper_compilation_token =
18911919
wasm::GetWasmEngine()->StartWrapperCompilation(isolate);
1892-
if (!wrapper_compilation_token) return;
1920+
if (!wrapper_compilation_token) return FlushRemainingUnits();
18931921
}
18941922

18951923
TRACE_EVENT0("v8.wasm", "wasm.JSToWasmWrapperCompilation");
@@ -1902,11 +1930,11 @@ class AsyncCompileJSToWasmWrapperJob final
19021930

19031931
BackgroundCompileScope compile_scope(native_module_);
19041932
if (compile_scope.cancelled()) return;
1905-
if (complete_last_unit)
1933+
if (complete_last_unit) {
19061934
compile_scope.compilation_state()->OnFinishedJSToWasmWrapperUnits();
1935+
}
19071936
if (yield) return;
1908-
size_t index = GetNextUnitIndex();
1909-
if (index >= compilation_units_size_) return;
1937+
if (!GetNextUnitIndex(&index)) return;
19101938
wrapper_unit =
19111939
compile_scope.compilation_state()->GetJSToWasmWrapperCompilationUnit(
19121940
index);
@@ -1916,8 +1944,6 @@ class AsyncCompileJSToWasmWrapperJob final
19161944
private:
19171945
std::weak_ptr<NativeModule> native_module_;
19181946
std::shared_ptr<OperationsBarrier> engine_barrier_;
1919-
// Number of wrappers to be compiled.
1920-
const size_t compilation_units_size_;
19211947
};
19221948

19231949
class BackgroundCompileJob final : public JobTask {
@@ -3723,8 +3749,9 @@ void CompilationStateImpl::WaitForCompilationEvent(
37233749
// Waiting on other CompilationEvent doesn't make sense.
37243750
UNREACHABLE();
37253751
}
3726-
if (js_to_wasm_wrapper_job_ && js_to_wasm_wrapper_job_->IsValid())
3752+
if (js_to_wasm_wrapper_job_ && js_to_wasm_wrapper_job_->IsValid()) {
37273753
js_to_wasm_wrapper_job_->Join();
3754+
}
37283755
#ifdef DEBUG
37293756
base::EnumSet<CompilationEvent> events{expect_event,
37303757
CompilationEvent::kFailedCompilation};
@@ -3786,9 +3813,8 @@ class CompileJSToWasmWrapperJob final : public BaseCompileJSToWasmWrapperJob {
37863813
compilation_units_(compilation_units) {}
37873814

37883815
void Run(JobDelegate* delegate) override {
3789-
while (true) {
3790-
size_t index = GetNextUnitIndex();
3791-
if (index >= compilation_units_->size()) return;
3816+
size_t index;
3817+
while (GetNextUnitIndex(&index)) {
37923818
JSToWasmWrapperCompilationUnit* unit =
37933819
(*compilation_units_)[index].second.get();
37943820
unit->Execute();

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL