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

vm: add run-after-evaluate microtask mode by addaleax · Pull Request #34023 · nodejs/node · GitHub

/ node Public

vm: add run-after-evaluate microtask mode - #34023

Closed
addaleax wants to merge 3 commits into
nodejs:masterfrom
addaleax:vm-3020
ConversationCommits (3)ChecksFiles changed
Closed

vm: add run-after-evaluate microtask mode#34023
addaleax wants to merge 3 commits into
nodejs:masterfrom
addaleax:vm-3020

Conversation

addaleax commented Jun 22, 2020

Copy link
Copy Markdown
Member

This allows timeouts to apply to e.g. Promises and async functions
from code running inside of vm.Contexts, by giving the Context its
own microtasks queue.

Fixes: #3020

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

vm: add run-after-evaluate microtask mode
35c4401 
This allows timeouts to apply to e.g. `Promise`s and `async function`s
from code running inside of `vm.Context`s, by giving the Context its
own microtasks queue.

Fixes: nodejs#3020
addaleax added vm Issues and PRs related to the vm subsystem. semver-minor PRs that contain new features and should be released in the next minor version. labels Jun 22, 2020
nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Jun 22, 2020

addaleax commented Jun 22, 2020

Copy link
Copy Markdown
Member Author

@nodejs/vm

addaleax removed the lib / src Issues and PRs related to general changes in the lib or src directory. label Jun 22, 2020

nodejs-github-bot commented Jun 22, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32039/

devsnek commented Jun 22, 2020

Copy link
Copy Markdown
Member

Flushing microtasks seems like a good approach, but i'm not sure why that requires MicrotaskQueueWrap to exist or ModuleWrap to have a ContextifyContext instance.

addaleax commented Jun 22, 2020

Copy link
Copy Markdown
Member Author

Flushing microtasks seems like a good approach, but i'm not sure why that requires MicrotaskQueueWrap to exist

Mostly because of the comment in the class definition – there’s no real reason not to give the caller full flexibility over running the microtask queue, it’s just out of scope here.

or ModuleWrap to have a ContextifyContext instance.

It’s absolutely possible that I have missed a simpler way to figure out the relevant microtask queue. If that is the case, please share it with me :)

devsnek commented Jun 23, 2020

Copy link
Copy Markdown
Member

I'm having trouble thinking of actual use cases for this. You still can't use any stdlib functions from node (or native addons) even if they use microtasks instead of node ticks. afaict anytime you would want to use this option, you should probably be using isolated-vm instead.

addaleax commented Jun 23, 2020

Copy link
Copy Markdown
Member Author

I'm having trouble thinking of actual use cases for this. You still can't use any stdlib functions from node (or native addons) even if they use microtasks instead of node ticks.

Fwiw, multi-context support for Node.js is not totally unthinkable at this point anymore. Quite a few of our internal bindings are set up in a way that would support that by now.

afaict anytime you would want to use this option, you should probably be using isolated-vm instead.

I mean, if you go that route, you might as well say that anybody who wants to use the timeout option should just use isolated-vm (or Worker threads). We have the option, though, and I’m not really happy about that fact, but I do think that this PR enables using it in more scenarios than previously.

nodejs-github-bot commented Jun 25, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32066/

lundibundi approved these changes Jun 26, 2020
View reviewed changes
Comment thread lib/vm.js
addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jun 26, 2020

This comment has been minimized.

Sign in to view
jasnell mentioned this pull request Jun 26, 2020
Closed
3 tasks

nodejs-github-bot commented Jun 26, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32106/

jasnell pushed a commit that referenced this pull request Jun 26, 2020
vm: add run-after-evaluate microtask mode
f63436d 
This allows timeouts to apply to e.g. `Promise`s and `async function`s
from code running inside of `vm.Context`s, by giving the Context its
own microtasks queue.

Fixes: #3020

PR-URL: #34023
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>

jasnell commented Jun 26, 2020

Copy link
Copy Markdown
Member

Landed in f63436d

jasnell closed this Jun 26, 2020
addaleax deleted the vm-3020 branch June 26, 2020 18:44
MylesBorins pushed a commit that referenced this pull request Jul 14, 2020
vm: add run-after-evaluate microtask mode
7dffcb3 
This allows timeouts to apply to e.g. `Promise`s and `async function`s
from code running inside of `vm.Context`s, by giving the Context its
own microtasks queue.

Fixes: #3020

PR-URL: #34023
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
MylesBorins added a commit that referenced this pull request Jul 14, 2020
2020-07-15, Version 14.6.0 (Current)
2e91d52 
Notable changes:

doc:
  * add danielleadams to collaborators (Danielle Adams) #34360
  * add sxa as collaborator (Stewart X Addison) #34338
  * add ruyadorno to collaborators (Ruy Adorno) #34297
src:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
  * (SEMVER-MINOR) allow embedders to disable esm loader (Shelley Vohr) #34060
tls:
  * (SEMVER-MINOR) make 'createSecureContext' honor more options (Mateusz Krawczuk) #33974
vm:
  * (SEMVER-MINOR) add run-after-evaluate microtask mode (Anna Henningsen) #34023
worker:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303

PR-URL: TODO
MylesBorins mentioned this pull request Jul 14, 2020
Merged
MylesBorins added a commit that referenced this pull request Jul 15, 2020
2020-07-16, Version 14.6.0 (Current)
6629373 
Notable changes:

deps:
  * upgrade npm to 6.14.6 (claudiahdz) #34246
  * upgrade to libuv 1.38.1 (Colin Ihrig) #34187
module:
  * (SEMVER-MINOR) deprecate module.parent (Antoine du HAMEL) #32217
  * (SEMVER-MINOR) package "imports" field (Guy Bedford) #34117
src:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
  * (SEMVER-MINOR) allow embedders to disable esm loader (Shelley Vohr) #34060
tls:
  * (SEMVER-MINOR) make 'createSecureContext' honor more options (Mateusz Krawczuk) #33974
vm:
  * (SEMVER-MINOR) add run-after-evaluate microtask mode (Anna Henningsen) #34023
worker:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
New Collaborators:
  * add danielleadams to collaborators (Danielle Adams) #34360
  * add sxa as collaborator (Stewart X Addison) #34338
  * add ruyadorno to collaborators (Ruy Adorno) #34297

PR-URL: #34371
MylesBorins added a commit that referenced this pull request Jul 16, 2020
2020-07-16, Version 14.6.0 (Current)
93694f6 
Notable changes:

deps:
  * upgrade npm to 6.14.6 (claudiahdz) #34246
  * upgrade to libuv 1.38.1 (Colin Ihrig) #34187
  * (SEMVER-MINOR) update V8 to 8.4.371.19 (Michaël Zasso) [#33579](#33579)
module:
  * (SEMVER-MINOR) deprecate module.parent (Antoine du HAMEL) #32217
  * (SEMVER-MINOR) package "imports" field (Guy Bedford) #34117
src:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
  * (SEMVER-MINOR) allow embedders to disable esm loader (Shelley Vohr) #34060
tls:
  * (SEMVER-MINOR) make 'createSecureContext' honor more options (Mateusz Krawczuk) #33974
vm:
  * (SEMVER-MINOR) add run-after-evaluate microtask mode (Anna Henningsen) #34023
worker:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
New Collaborators:
  * add danielleadams to collaborators (Danielle Adams) #34360
  * add sxa as collaborator (Stewart X Addison) #34338
  * add ruyadorno to collaborators (Ruy Adorno) #34297

PR-URL: #34371
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
vm: add run-after-evaluate microtask mode
70c4045 
This allows timeouts to apply to e.g. `Promise`s and `async function`s
from code running inside of `vm.Context`s, by giving the Context its
own microtasks queue.

Fixes: #3020

PR-URL: #34023
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
MylesBorins added a commit that referenced this pull request Jul 16, 2020
2020-07-16, Version 14.6.0 (Current)
35a4b7b 
Notable changes:

deps:
  * upgrade npm to 6.14.6 (claudiahdz) #34246
  * upgrade to libuv 1.38.1 (Colin Ihrig) #34187
  * (SEMVER-MINOR) update V8 to 8.4.371.19 (Michaël Zasso) [#33579](#33579)
module:
  * (SEMVER-MINOR) deprecate module.parent (Antoine du HAMEL) #32217
  * (SEMVER-MINOR) package "imports" field (Guy Bedford) #34117
src:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
  * (SEMVER-MINOR) allow embedders to disable esm loader (Shelley Vohr) #34060
tls:
  * (SEMVER-MINOR) make 'createSecureContext' honor more options (Mateusz Krawczuk) #33974
vm:
  * (SEMVER-MINOR) add run-after-evaluate microtask mode (Anna Henningsen) #34023
worker:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
New Collaborators:
  * add danielleadams to collaborators (Danielle Adams) #34360
  * add sxa as collaborator (Stewart X Addison) #34338
  * add ruyadorno to collaborators (Ruy Adorno) #34297

PR-URL: #34371
MylesBorins added a commit that referenced this pull request Jul 20, 2020
2020-07-21, Version 14.6.0 (Current)
049631f 
Notable changes:

deps:
  * upgrade npm to 6.14.6 (claudiahdz) #34246
  * upgrade to libuv 1.38.1 (Colin Ihrig) #34187
  * (SEMVER-MINOR) update V8 to 8.4.371.19 (Michaël Zasso) [#33579](#33579)
module:
  * (SEMVER-MINOR) deprecate module.parent (Antoine du HAMEL) #32217
  * (SEMVER-MINOR) package "imports" field (Guy Bedford) #34117
src:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
  * (SEMVER-MINOR) allow embedders to disable esm loader (Shelley Vohr) #34060
tls:
  * (SEMVER-MINOR) make 'createSecureContext' honor more options (Mateusz Krawczuk) #33974
vm:
  * (SEMVER-MINOR) add run-after-evaluate microtask mode (Anna Henningsen) #34023
worker:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
New Collaborators:
  * add danielleadams to collaborators (Danielle Adams) #34360
  * add sxa as collaborator (Stewart X Addison) #34338
  * add ruyadorno to collaborators (Ruy Adorno) #34297

PR-URL: #34371
MylesBorins added a commit that referenced this pull request Jul 20, 2020
2020-07-21, Version 14.6.0 (Current)
7999aa2 
Notable changes:

deps:
  * upgrade npm to 6.14.6 (claudiahdz) #34246
  * upgrade to libuv 1.38.1 (Colin Ihrig) #34187
  * (SEMVER-MINOR) update V8 to 8.4.371.19 (Michaël Zasso) [#33579](#33579)
module:
  * (SEMVER-MINOR) deprecate module.parent (Antoine du HAMEL) #32217
  * (SEMVER-MINOR) package "imports" field (Guy Bedford) #34117
src:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
  * (SEMVER-MINOR) allow embedders to disable esm loader (Shelley Vohr) #34060
tls:
  * (SEMVER-MINOR) make 'createSecureContext' honor more options (Mateusz Krawczuk) #33974
vm:
  * (SEMVER-MINOR) add run-after-evaluate microtask mode (Anna Henningsen) #34023
worker:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
New Collaborators:
  * add danielleadams to collaborators (Danielle Adams) #34360
  * add sxa as collaborator (Stewart X Addison) #34338
  * add ruyadorno to collaborators (Ruy Adorno) #34297

PR-URL: #34371
MylesBorins added a commit that referenced this pull request Jul 20, 2020
2020-07-21, Version 14.6.0 (Current)
ca62c34 
Notable changes:

deps:
  * upgrade npm to 6.14.6 (claudiahdz) #34246
  * upgrade to libuv 1.38.1 (Colin Ihrig) #34187
  * (SEMVER-MINOR) update V8 to 8.4.371.19 (Michaël Zasso) [#33579](#33579)
module:
  * (SEMVER-MINOR) deprecate module.parent (Antoine du HAMEL) #32217
  * (SEMVER-MINOR) package "imports" field (Guy Bedford) #34117
src:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
  * (SEMVER-MINOR) allow embedders to disable esm loader (Shelley Vohr) #34060
tls:
  * (SEMVER-MINOR) make 'createSecureContext' honor more options (Mateusz Krawczuk) #33974
vm:
  * (SEMVER-MINOR) add run-after-evaluate microtask mode (Anna Henningsen) #34023
worker:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
New Collaborators:
  * add danielleadams to collaborators (Danielle Adams) #34360
  * add sxa as collaborator (Stewart X Addison) #34338
  * add ruyadorno to collaborators (Ruy Adorno) #34297

PR-URL: #34371
MylesBorins added a commit that referenced this pull request Jul 21, 2020
2020-07-21, Version 14.6.0 (Current)
a981c65 
Notable changes:

deps:
  * upgrade npm to 6.14.6 (claudiahdz) #34246
  * upgrade to libuv 1.38.1 (Colin Ihrig) #34187
  * (SEMVER-MINOR) update V8 to 8.4.371.19 (Michaël Zasso) [#33579](#33579)
module:
  * (SEMVER-MINOR) deprecate module.parent (Antoine du HAMEL) #32217
  * (SEMVER-MINOR) package "imports" field (Guy Bedford) #34117
src:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
  * (SEMVER-MINOR) allow embedders to disable esm loader (Shelley Vohr) #34060
tls:
  * (SEMVER-MINOR) make 'createSecureContext' honor more options (Mateusz Krawczuk) #33974
vm:
  * (SEMVER-MINOR) add run-after-evaluate microtask mode (Anna Henningsen) #34023
worker:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
New Collaborators:
  * add danielleadams to collaborators (Danielle Adams) #34360
  * add sxa as collaborator (Stewart X Addison) #34338
  * add ruyadorno to collaborators (Ruy Adorno) #34297

PR-URL: #34371
cjihrig pushed a commit that referenced this pull request Jul 23, 2020
2020-07-21, Version 14.6.0 (Current)
38678ef 
Notable changes:

deps:
  * upgrade npm to 6.14.6 (claudiahdz) #34246
  * upgrade to libuv 1.38.1 (Colin Ihrig) #34187
  * (SEMVER-MINOR) update V8 to 8.4.371.19 (Michaël Zasso) [#33579](#33579)
module:
  * (SEMVER-MINOR) deprecate module.parent (Antoine du HAMEL) #32217
  * (SEMVER-MINOR) package "imports" field (Guy Bedford) #34117
src:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
  * (SEMVER-MINOR) allow embedders to disable esm loader (Shelley Vohr) #34060
tls:
  * (SEMVER-MINOR) make 'createSecureContext' honor more options (Mateusz Krawczuk) #33974
vm:
  * (SEMVER-MINOR) add run-after-evaluate microtask mode (Anna Henningsen) #34023
worker:
  * (SEMVER-MINOR) add option to track unmanaged file descriptors (Anna Henningsen) #34303
New Collaborators:
  * add danielleadams to collaborators (Danielle Adams) #34360
  * add sxa as collaborator (Stewart X Addison) #34338
  * add ruyadorno to collaborators (Ruy Adorno) #34297

PR-URL: #34371
exoego added a commit to exoego/scala-js-nodejs that referenced this pull request Jul 30, 2020
Ref. nodejs/node#34023
65f9564
Raoul1996 mentioned this pull request Oct 15, 2020
Closed
koal44 mentioned this pull request Aug 26, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
+1 more reviewer
lundibundi lundibundi approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. semver-minor PRs that contain new features and should be released in the next minor version. vm Issues and PRs related to the vm subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Promises allow vm.runInContext timeout to be escaped

5 participants

Back | FazBrowse Home | New Git URL