Skip to content

Commit

Permalink
Integration test for CDPAgent reentrancy bugs
Browse files Browse the repository at this point in the history
Summary:
Changelog: [Internal]

Adds a regression test for a Hermes CDPAgent bug using `JsiIntegrationTest`. See details in comments.

bypass-github-export-checks

Reviewed By: huntie

Differential Revision: D54890188

fbshipit-source-id: 325a32b83b145d2d35f99feaef9988028f15f196
  • Loading branch information
motiz88 authored and facebook-github-bot committed Mar 14, 2024
1 parent 0d7a92b commit 4148a31
Showing 1 changed file with 60 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,66 @@ TYPED_TEST(JsiIntegrationHermesModernTest, ResolveBreakpointAfterReload) {
scriptInfo->value()["params"]["scriptId"]);
}

TYPED_TEST(JsiIntegrationHermesModernTest, CDPAgentReentrancyRegressionTest) {
this->connect();

// TODO(moti): Add an InSequence guard here once Hermes processes the messages
// in the order they were received.

this->inspectorExecutor_([&]() {
// Tasks scheduled on our executor here will be executed when this lambda
// returns. This is integral to the bug we're trying to reproduce, so we
// place the EXPECT_* calls at the end of the lambda body to ensure the
// test fails if we get eager (unexpected) responses.

// 1. Cause CDPAgent to schedule a task on the JS executor to process the
// message. The task is simultaneously scheduled as an interrupt on the
// JS VM (but will be called via the executor, since the interpreter is
// idle at the moment).
this->toPage_->sendMessage(R"({
"id": 1,
"method": "Runtime.evaluate",
"params": {"expression": "1 + 2"}
})");

// 2. Cause CDPAgent to schedule another task+interrupt.
this->toPage_->sendMessage(R"({
"id": 2,
"method": "Runtime.evaluate",
"params": {"expression": "3 + 4"}
})");

// 3. When the first scheduled task runs (via the executor), it enters the
// interpreter to evaluate the expression, at which point the
// interpreter begins processing interrupts and enters the second task.
// This used to trigger two distinct bugs in CDPAgent:
// - The first task would be triggered twice due to a race condition
// between the executor and the interrupt handler. (D54771697)
// - The second task would deadlock due to the first task holding a
// lock preventing any other CDPAgent tasks from running. (D54838179)

this->expectMessageFromPage(JsonEq(R"({
"id": 1,
"result": {
"result": {
"type": "number",
"value": 3
}
}
})"));

this->expectMessageFromPage(JsonEq(R"({
"id": 2,
"result": {
"result": {
"type": "number",
"value": 7
}
}
})"));
});
}

#pragma endregion // ModernHermesVariants

} // namespace facebook::react::jsinspector_modern

0 comments on commit 4148a31

Please sign in to comment.