mirror of
https://github.com/nodejs/node.git
synced 2026-10-28 10:15:07 +08:00
deps: V8: backport 5177b10891e6
Original commit message:
fix(inspector): hold on to promises
Keep `m_evaluationResult` strong for evaluations
until the promise settles or the request is cancelled.
Bug: 536271637
Change-Id: If21cc4aa0ba6bb2e2722d5ee73eb7744a0ead207
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8123081
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Reviewed-by: Kim-Anh Tran <kimanh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#108874}
Refs: https://github.com/v8/v8/commit/5177b10891e65108c1a19dfc56bff4e58d79d216
Co-authored-by: avivkeller <me@aviv.sh>
PR-URL: https://github.com/nodejs/node/pull/64631
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
This commit is contained in:
1 parent
b7d29fee74
commit
a0d1911bee
8 files changed
+218
-7
No files matched your search
+1
-1
@@ -42,7 +42,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.27',
|
||||
'v8_embedder_string': '-node.28',
|
||||
|
||||
##### V8 defaults for Node.js #####
|
||||
|
||||
|
||||
Vendored
+1
@@ -77,6 +77,7 @@ Artem Kobzar <artem.kobzar@jetbrains.com>
|
||||
Arthur Islamov <arthur@islamov.ai>
|
||||
Asuka Shikina <shikina.asuka@gmail.com>
|
||||
Aurèle Barrière <aurele.barriere@gmail.com>
|
||||
Aviv Keller <me@aviv.sh>
|
||||
Bala Avulapati <bavulapati@gmail.com>
|
||||
Bangfu Tao <bangfu.tao@samsung.com>
|
||||
Ben Coe <bencoe@gmail.com>
|
||||
|
||||
+37
-6
@@ -204,8 +204,7 @@ class InjectedScript::ProtocolPromiseHandler {
|
||||
PromiseHandlerTracker::DiscardReason::kFulfilled);
|
||||
}
|
||||
|
||||
ProtocolPromiseHandler(PromiseHandlerTracker::Id id,
|
||||
V8InspectorSessionImpl* session,
|
||||
ProtocolPromiseHandler(V8InspectorSessionImpl* session,
|
||||
int executionContextId, const String16& objectGroup,
|
||||
std::unique_ptr<WrapOptions> wrapOptions,
|
||||
bool replMode, bool throwOnSideEffect,
|
||||
@@ -220,7 +219,13 @@ class InjectedScript::ProtocolPromiseHandler {
|
||||
m_replMode(replMode),
|
||||
m_throwOnSideEffect(throwOnSideEffect),
|
||||
m_callback(std::move(callback)),
|
||||
m_evaluationResult(m_inspector->isolate(), evaluationResult) {
|
||||
m_evaluationResult(m_inspector->isolate(), evaluationResult) {}
|
||||
|
||||
void makeWeak(PromiseHandlerTracker::Id id) {
|
||||
if (m_isActive || m_evaluationResult.IsEmpty() ||
|
||||
m_evaluationResult.IsWeak()) {
|
||||
return;
|
||||
}
|
||||
m_evaluationResult.SetWeak(reinterpret_cast<PromiseHandlerTracker::Id*>(id),
|
||||
cleanup, v8::WeakCallbackType::kParameter);
|
||||
}
|
||||
@@ -238,6 +243,7 @@ class InjectedScript::ProtocolPromiseHandler {
|
||||
}
|
||||
|
||||
void thenCallback(v8::Local<v8::Value> value) {
|
||||
m_isActive = true;
|
||||
// We don't need the m_evaluationResult in the `thenCallback`, but we also
|
||||
// don't want `cleanup` running in case we re-enter JS.
|
||||
m_evaluationResult.Reset();
|
||||
@@ -285,9 +291,10 @@ class InjectedScript::ProtocolPromiseHandler {
|
||||
}
|
||||
|
||||
void catchCallback(v8::Local<v8::Value> result) {
|
||||
m_isActive = true;
|
||||
// Hold strongly onto m_evaluationResult now to prevent `cleanup` from
|
||||
// running in case any code below triggers GC.
|
||||
m_evaluationResult.ClearWeak();
|
||||
if (m_evaluationResult.IsWeak()) m_evaluationResult.ClearWeak();
|
||||
V8InspectorSessionImpl* session =
|
||||
m_inspector->sessionById(m_contextGroupId, m_sessionId);
|
||||
if (!session) return;
|
||||
@@ -393,6 +400,7 @@ class InjectedScript::ProtocolPromiseHandler {
|
||||
std::unique_ptr<WrapOptions> m_wrapOptions;
|
||||
bool m_replMode;
|
||||
bool m_throwOnSideEffect;
|
||||
bool m_isActive = false;
|
||||
std::weak_ptr<EvaluateCallback> m_callback;
|
||||
v8::Global<v8::Promise> m_evaluationResult;
|
||||
};
|
||||
@@ -1190,8 +1198,7 @@ template <typename... Args>
|
||||
PromiseHandlerTracker::Id PromiseHandlerTracker::create(Args&&... args) {
|
||||
Id id = m_lastUsedId++;
|
||||
InjectedScript::ProtocolPromiseHandler* handler =
|
||||
new InjectedScript::ProtocolPromiseHandler(id,
|
||||
std::forward<Args>(args)...);
|
||||
new InjectedScript::ProtocolPromiseHandler(std::forward<Args>(args)...);
|
||||
m_promiseHandlers.emplace(id, handler);
|
||||
return id;
|
||||
}
|
||||
@@ -1225,6 +1232,30 @@ InjectedScript::ProtocolPromiseHandler* PromiseHandlerTracker::get(
|
||||
return iter->second.get();
|
||||
}
|
||||
|
||||
void PromiseHandlerTracker::makeWeakForContext(int executionContextId) {
|
||||
for (auto& [id, handler] : m_promiseHandlers) {
|
||||
if (handler->m_executionContextId == executionContextId) {
|
||||
handler->makeWeak(id);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void PromiseHandlerTracker::makeWeakForObjectGroup(
|
||||
int sessionId, const String16& objectGroup) {
|
||||
for (auto& [id, handler] : m_promiseHandlers) {
|
||||
if (handler->m_sessionId == sessionId &&
|
||||
handler->m_objectGroup == objectGroup) {
|
||||
handler->makeWeak(id);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void PromiseHandlerTracker::makeWeakForSession(int sessionId) {
|
||||
for (auto& [id, handler] : m_promiseHandlers) {
|
||||
if (handler->m_sessionId == sessionId) handler->makeWeak(id);
|
||||
}
|
||||
}
|
||||
|
||||
void PromiseHandlerTracker::sendFailure(
|
||||
InjectedScript::ProtocolPromiseHandler* handler,
|
||||
const protocol::DispatchResponse& response) const {
|
||||
|
||||
+3
@@ -298,6 +298,9 @@ class PromiseHandlerTracker {
|
||||
Id create(Args&&... args);
|
||||
void discard(Id id, DiscardReason reason);
|
||||
InjectedScript::ProtocolPromiseHandler* get(Id id) const;
|
||||
void makeWeakForContext(int executionContextId);
|
||||
void makeWeakForObjectGroup(int sessionId, const String16& objectGroup);
|
||||
void makeWeakForSession(int sessionId);
|
||||
|
||||
private:
|
||||
void sendFailure(InjectedScript::ProtocolPromiseHandler* handler,
|
||||
|
||||
+1
@@ -319,6 +319,7 @@ void V8InspectorImpl::contextCollected(int groupId, int contextId) {
|
||||
session->runtimeAgent()->reportExecutionContextDestroyed(inspectedContext);
|
||||
});
|
||||
discardInspectedContext(groupId, contextId);
|
||||
m_promiseHandlerTracker.makeWeakForContext(contextId);
|
||||
}
|
||||
|
||||
void V8InspectorImpl::resetContextGroup(int contextGroupId) {
|
||||
|
||||
@@ -224,6 +224,7 @@ void V8InspectorSessionImpl::discardInjectedScripts() {
|
||||
[&sessionId](InspectedContext* context) {
|
||||
context->discardInjectedScript(sessionId);
|
||||
});
|
||||
m_inspector->promiseHandlerTracker().makeWeakForSession(sessionId);
|
||||
}
|
||||
|
||||
Response V8InspectorSessionImpl::findInjectedScript(
|
||||
@@ -260,6 +261,10 @@ void V8InspectorSessionImpl::releaseObjectGroup(const String16& objectGroup) {
|
||||
InjectedScript* injectedScript = context->getInjectedScript(sessionId);
|
||||
if (injectedScript) injectedScript->releaseObjectGroup(objectGroup);
|
||||
});
|
||||
if (!objectGroup.isEmpty()) {
|
||||
m_inspector->promiseHandlerTracker().makeWeakForObjectGroup(m_sessionId,
|
||||
objectGroup);
|
||||
}
|
||||
}
|
||||
|
||||
bool V8InspectorSessionImpl::unwrapObject(
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
Tests the lifetime of pending Runtime.evaluate requests.
|
||||
|
||||
Running test: testPromiseIsKeptAlive
|
||||
Using replMode:
|
||||
{
|
||||
id : <messageId>
|
||||
result : {
|
||||
result : {
|
||||
description : 42
|
||||
type : number
|
||||
value : 42
|
||||
}
|
||||
}
|
||||
}
|
||||
Using awaitPromise:
|
||||
{
|
||||
id : <messageId>
|
||||
result : {
|
||||
result : {
|
||||
description : 42
|
||||
type : number
|
||||
value : 42
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Running test: testObjectGroupReleaseMakesPromiseCollectible
|
||||
Using replMode:
|
||||
{
|
||||
error : {
|
||||
code : -32000
|
||||
message : Promise was collected
|
||||
}
|
||||
id : <messageId>
|
||||
}
|
||||
Using awaitPromise:
|
||||
{
|
||||
error : {
|
||||
code : -32000
|
||||
message : Promise was collected
|
||||
}
|
||||
id : <messageId>
|
||||
}
|
||||
|
||||
Running test: testContextDestructionDiscardsPromise
|
||||
Using replMode:
|
||||
{
|
||||
error : {
|
||||
code : -32000
|
||||
message : Execution context was destroyed.
|
||||
}
|
||||
id : <messageId>
|
||||
}
|
||||
Using awaitPromise:
|
||||
{
|
||||
error : {
|
||||
code : -32000
|
||||
message : Execution context was destroyed.
|
||||
}
|
||||
id : <messageId>
|
||||
}
|
||||
|
||||
Running test: testSessionDestructionMakesPromiseCollectible
|
||||
Promise is alive before disconnect: true
|
||||
Promise is alive after disconnect: false
|
||||
@@ -0,0 +1,105 @@
|
||||
// Copyright 2026 the V8 project authors. All rights reserved.
|
||||
// Use of this source code is governed by a BSD-style license that can be
|
||||
// found in the LICENSE file.
|
||||
|
||||
// Flags: --no-stress-incremental-marking
|
||||
|
||||
let {Protocol} = InspectorTest.start(
|
||||
'Tests the lifetime of pending Runtime.evaluate requests.');
|
||||
|
||||
const evaluationModes = [
|
||||
{
|
||||
name: 'replMode',
|
||||
arguments: {replMode: true},
|
||||
expression:
|
||||
'await new Promise(resolve => globalThis.resolve = resolve); 42',
|
||||
resolveExpression: 'resolve()',
|
||||
pendingExpression: 'await new Promise(() => {})',
|
||||
},
|
||||
{
|
||||
name: 'awaitPromise',
|
||||
arguments: {awaitPromise: true},
|
||||
expression: `(() => {
|
||||
let resolve;
|
||||
const promise = new Promise(r => resolve = r);
|
||||
promise.resolve = resolve;
|
||||
globalThis.weak = new WeakRef(promise);
|
||||
return promise;
|
||||
})()`,
|
||||
resolveExpression: 'weak.deref().resolve(42)',
|
||||
pendingExpression: 'new Promise(() => {})',
|
||||
},
|
||||
];
|
||||
|
||||
function evaluate(Protocol, mode, expression, extraArguments = {}) {
|
||||
return Protocol.Runtime.evaluate(
|
||||
{...mode.arguments, ...extraArguments, expression});
|
||||
}
|
||||
|
||||
InspectorTest.runAsyncTestSuite([
|
||||
async function testPromiseIsKeptAlive() {
|
||||
for (const mode of evaluationModes) {
|
||||
InspectorTest.log(`Using ${mode.name}:`);
|
||||
const evaluation = evaluate(Protocol, mode, mode.expression);
|
||||
|
||||
await Protocol.HeapProfiler.collectGarbage();
|
||||
await Protocol.Runtime.evaluate({expression: mode.resolveExpression});
|
||||
|
||||
InspectorTest.logMessage(await evaluation);
|
||||
}
|
||||
},
|
||||
|
||||
async function testObjectGroupReleaseMakesPromiseCollectible() {
|
||||
for (const mode of evaluationModes) {
|
||||
InspectorTest.log(`Using ${mode.name}:`);
|
||||
const evaluation = evaluate(
|
||||
Protocol, mode, mode.pendingExpression,
|
||||
{objectGroup: 'evaluation'});
|
||||
|
||||
await Protocol.Runtime.releaseObjectGroup({objectGroup: 'evaluation'});
|
||||
await Protocol.HeapProfiler.collectGarbage();
|
||||
|
||||
InspectorTest.logMessage(await evaluation);
|
||||
}
|
||||
},
|
||||
|
||||
async function testContextDestructionDiscardsPromise() {
|
||||
for (const mode of evaluationModes) {
|
||||
InspectorTest.log(`Using ${mode.name}:`);
|
||||
const contextGroup = new InspectorTest.ContextGroup();
|
||||
const session = contextGroup.connect();
|
||||
const evaluation = evaluate(
|
||||
session.Protocol, mode, mode.pendingExpression);
|
||||
|
||||
await session.Protocol.Runtime.evaluate(
|
||||
{expression: 'inspector.fireContextDestroyed()'});
|
||||
|
||||
InspectorTest.logMessage(await evaluation);
|
||||
session.disconnect();
|
||||
}
|
||||
},
|
||||
|
||||
async function testSessionDestructionMakesPromiseCollectible() {
|
||||
const contextGroup = new InspectorTest.ContextGroup();
|
||||
const session1 = contextGroup.connect();
|
||||
const session2 = contextGroup.connect();
|
||||
session1.Protocol.Runtime.evaluate({
|
||||
expression: evaluationModes[1].expression,
|
||||
awaitPromise: true,
|
||||
});
|
||||
|
||||
await session2.Protocol.HeapProfiler.collectGarbage();
|
||||
let result = await session2.Protocol.Runtime.evaluate(
|
||||
{expression: 'weak.deref() !== undefined'});
|
||||
InspectorTest.log(
|
||||
`Promise is alive before disconnect: ${result.result.result.value}`);
|
||||
|
||||
session1.disconnect();
|
||||
await session2.Protocol.HeapProfiler.collectGarbage();
|
||||
result = await session2.Protocol.Runtime.evaluate(
|
||||
{expression: 'weak.deref() !== undefined'});
|
||||
InspectorTest.log(
|
||||
`Promise is alive after disconnect: ${result.result.result.value}`);
|
||||
session2.disconnect();
|
||||
},
|
||||
]);
|
||||
Reference in new issue
Block a user