vm: fix race condition with `timeout` param by laverdet · Pull Request #13074 · nodejs/node

vm: fix race condition with `timeout` param by laverdet · Pull Request #13074 · nodejs/node

added c++

Issues and PRs that require attention from people who are familiar with C++.

Issues and PRs related to the REPL subsystem.

Issues and PRs related to the vm subsystem.

labels

This option has been broken for almost a year when used with any of the
vm.runIn.. family of functions, except for syntax errors.

This fixes a race condition in the watchdog timer used for vm timeouts.
The condition would terminate the main stack's execution instead of the
code running under the sandbox.

addaleax pushed a commit that referenced this pull request

This option has been broken for almost a year when used with any of the
vm.runIn.. family of functions, except for syntax errors.

PR-URL: #13074
Reviewed-By: Anna Henningsen <anna@addaleax.net>

addaleax pushed a commit that referenced this pull request

This fixes a race condition in the watchdog timer used for vm timeouts.
The condition would terminate the main stack's execution instead of the
code running under the sandbox.

PR-URL: #13074
Reviewed-By: Anna Henningsen <anna@addaleax.net>

jasnell pushed a commit that referenced this pull request

This option has been broken for almost a year when used with any of the
vm.runIn.. family of functions, except for syntax errors.

PR-URL: #13074
Reviewed-By: Anna Henningsen <anna@addaleax.net>

jasnell pushed a commit that referenced this pull request

This fixes a race condition in the watchdog timer used for vm timeouts.
The condition would terminate the main stack's execution instead of the
code running under the sandbox.

PR-URL: #13074
Reviewed-By: Anna Henningsen <anna@addaleax.net>

kunalspathak added a commit to kunalspathak/node-chakracore that referenced this pull request

* test_constructor: Napi test was failing because of over specifiying
a not condition. Fixed that and also restructured the code to not
iterate over list of properties twice.

* test-vm-timeout-rethrow: This started failing after
nodejs/node#13074 changes upstream. The change is
to not do `GetAndClearException` but allocate a string (`JsCreateString`)
which throws because engine is in exception state. We need to figure out
better way to handle these situation. Marked this as flaky for now.

* test-bindings: Marked this as flaky for now.

kunalspathak added a commit to kunalspathak/node-chakracore that referenced this pull request

* test_constructor: Napi test was failing because of over specifiying
a not condition. Fixed that and also restructured the code to not
iterate over list of properties twice.

* test-vm-timeout-rethrow: This started failing after
nodejs/node#13074 changes upstream. The change is
to not do `GetAndClearException` but allocate a string (`JsCreateString`)
which throws because engine is in exception state. We need to figure out
better way to handle these situation. Marked this as flaky for now.

* test-bindings: Marked this as flaky for now.

kunalspathak added a commit to kunalspathak/node-chakracore that referenced this pull request

* test_constructor: Napi test was failing because of over specifiying
a not condition. Fixed that and also restructured the code to not
iterate over list of properties twice.

* test-vm-timeout-rethrow: This started failing after
nodejs/node#13074 changes upstream. The change is
to not do `GetAndClearException` but allocate a string (`JsCreateString`)
which throws because engine is in exception state. We need to figure out
better way to handle these situation. Marked this as flaky for now.

* test-bindings: Marked this as flaky for now.

kunalspathak added a commit to kunalspathak/node-chakracore that referenced this pull request

* test-vm-timeout-rethrow: This started failing after
nodejs/node#13074/ changes upstream. The change is
to not do `GetAndClearException` but allocate a string (`JsCreateString`)
which throws because engine is in exception state. We need to figure out
better way to handle these situation. Marked this as flaky for now.

* test-bindings: Marked this as flaky for now.

PR-URL: nodejs#264
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>

kunalspathak added a commit to kunalspathak/node-chakracore that referenced this pull request

* test-vm-timeout-rethrow: This started failing after
nodejs/node#13074/ changes upstream. The change is
to not do `GetAndClearException` but allocate a string (`JsCreateString`)
which throws because engine is in exception state. We need to figure out
better way to handle these situation. Marked below test as flaky.
* parallel/test-vm-sigint : Marked this as flaky for now.
* parallel/test-vm-sigint-existing-handler : Marked this as flaky for now.
* parallel/test-repl-sigint : Marked this as flaky for now.

* parallel/test-bindings: Marked this as flaky for now.

PR-URL: nodejs#264
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>

jasnell pushed a commit that referenced this pull request

This option has been broken for almost a year when used with any of the
vm.runIn.. family of functions, except for syntax errors.

PR-URL: #13074
Reviewed-By: Anna Henningsen <anna@addaleax.net>

jasnell pushed a commit that referenced this pull request

This fixes a race condition in the watchdog timer used for vm timeouts.
The condition would terminate the main stack's execution instead of the
code running under the sandbox.

PR-URL: #13074
Reviewed-By: Anna Henningsen <anna@addaleax.net>

kfarnung pushed a commit to kfarnung/node-chakracore that referenced this pull request

* test-vm-timeout-rethrow: This started failing after
nodejs/node#13074/ changes upstream. The change is
to not do `GetAndClearException` but allocate a string (`JsCreateString`)
which throws because engine is in exception state. We need to figure out
better way to handle these situation. Marked below test as flaky.
* parallel/test-vm-sigint : Marked this as flaky for now.
* parallel/test-vm-sigint-existing-handler : Marked this as flaky for now.
* parallel/test-repl-sigint : Marked this as flaky for now.

* parallel/test-bindings: Marked this as flaky for now.

PR-URL: nodejs#264
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>

kfarnung pushed a commit to kfarnung/node-chakracore that referenced this pull request

* test-vm-timeout-rethrow: This started failing after
nodejs/node#13074/ changes upstream. The change is
to not do `GetAndClearException` but allocate a string (`JsCreateString`)
which throws because engine is in exception state. We need to figure out
better way to handle these situation. Marked below test as flaky.
* parallel/test-vm-sigint : Marked this as flaky for now.
* parallel/test-vm-sigint-existing-handler : Marked this as flaky for now.
* parallel/test-repl-sigint : Marked this as flaky for now.

* parallel/test-bindings: Marked this as flaky for now.

PR-URL: nodejs#264
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>

kfarnung pushed a commit to kfarnung/node-chakracore that referenced this pull request

* test-vm-timeout-rethrow: This started failing after
nodejs/node#13074/ changes upstream. The change is
to not do `GetAndClearException` but allocate a string (`JsCreateString`)
which throws because engine is in exception state. We need to figure out
better way to handle these situation. Marked below test as flaky.
* parallel/test-vm-sigint : Marked this as flaky for now.
* parallel/test-vm-sigint-existing-handler : Marked this as flaky for now.
* parallel/test-repl-sigint : Marked this as flaky for now.

* parallel/test-bindings: Marked this as flaky for now.

PR-URL: nodejs#264
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>

kfarnung pushed a commit to kfarnung/node-chakracore that referenced this pull request

* test-vm-timeout-rethrow: This started failing after
nodejs/node#13074/ changes upstream. The change is
to not do `GetAndClearException` but allocate a string (`JsCreateString`)
which throws because engine is in exception state. We need to figure out
better way to handle these situation. Marked below test as flaky.
* parallel/test-vm-sigint : Marked this as flaky for now.
* parallel/test-vm-sigint-existing-handler : Marked this as flaky for now.
* parallel/test-repl-sigint : Marked this as flaky for now.

* parallel/test-bindings: Marked this as flaky for now.

PR-URL: nodejs#264
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>

laverdet added a commit to laverdet/node that referenced this pull request

This option has been broken for almost a year when used with any of the
vm.runIn.. family of functions, except for syntax errors.

PR-URL: nodejs#13074
Reviewed-By: Anna Henningsen <anna@addaleax.net>

laverdet added a commit to laverdet/node that referenced this pull request

This fixes a race condition in the watchdog timer used for vm timeouts.
The condition would terminate the main stack's execution instead of the
code running under the sandbox.

PR-URL: nodejs#13074
Reviewed-By: Anna Henningsen <anna@addaleax.net>

MylesBorins pushed a commit that referenced this pull request

This option has been broken for almost a year when used with any of the
vm.runIn.. family of functions, except for syntax errors.

Backport-PR-URL: #14373
PR-URL: #13074
Reviewed-By: Anna Henningsen <anna@addaleax.net>

MylesBorins pushed a commit that referenced this pull request

This fixes a race condition in the watchdog timer used for vm timeouts.
The condition would terminate the main stack's execution instead of the
code running under the sandbox.

Backport-PR-URL: #14373
PR-URL: #13074
Reviewed-By: Anna Henningsen <anna@addaleax.net>

MylesBorins pushed a commit that referenced this pull request

This option has been broken for almost a year when used with any of the
vm.runIn.. family of functions, except for syntax errors.

Backport-PR-URL: #14373
PR-URL: #13074
Reviewed-By: Anna Henningsen <anna@addaleax.net>

MylesBorins pushed a commit that referenced this pull request

This fixes a race condition in the watchdog timer used for vm timeouts.
The condition would terminate the main stack's execution instead of the
code running under the sandbox.

Backport-PR-URL: #14373
PR-URL: #13074
Reviewed-By: Anna Henningsen <anna@addaleax.net>