Repository navigation
Promises allow vm.runInContext timeout to be escaped #3020
Description
Activity
I added the original
vmtimeout capability. This gets pretty nasty.. the watchdog timer can only cover the execution time of the actual script passed in. If that script, during its execution, interacts with the Node/uvevent queue and queues up more asynchronous callbacks (process.nextTick,setTimeout,setInterval) then those will execute later and the watchdog object will have already been destructed if the script executed in less time than the timeout.One possible way this could be implemented is if the
libuvcould support something like mentioned in the comments interposed into the actualvmcode:if (timeout != -1) { // query and save uv main loop reference count Watchdog wd(env, timeout); result = script->Run(); // keep spinning uv event loop until reference count equals saved value (soft blocking) } else {If the main loop ref count is equal to begin with, it will not block, the watchdog will destruct and everything will be fine. If the ref count is not equal, then that should indicate outstanding events have a ref on the loop and blocking should allow the watchdog to work as intended.
A side effect from this change would be that code setting up an indefinite listener would always hit the timeout. You would only be able to execute code that completely and fully finishes executing, including any interaction(s) with the event loop.
@bnoordhuis can you comment on the possibility of doing something like what I mentioned above in
uv? I'm just throwing out that idea, but there may be other reasons why that is not possible. The only other way I could think of is very nasty and involves injecting wrappers for any functions that can add to the event queue and force a check against some pre-computed timestamp.We can probably handle this by triggering microtask queue flush after
vminvocation, though it uses C++ APIs so I'm not sure it can be interrupted properly.All contexts share the same microtask queue (it's a per-isolate property) so I don't think this can be easily fixed, the promise callbacks can always enqueue new callbacks, ad infinitum.
@bnoordhuis right, there is now way to flush only microtask added inside of the new context. didn't think of that.
@bnoordhuis what I was thinking was that outstanding callbacks would ref the
uvevent loop, no? So if a way was added to block until ref count reached a desired value, then it would allow the main thread to block, thus allowing the watchdog to do its thing. If the code kept enqueuing callbacks and the count never reached the initial value then it would rightfully timeout, but behavior outside of thevmtimeout case would not be affected in any way.I don't think that could work. Contexts share the event loop. You could wait1 until the event loop's reference count drops below a certain threshold but that doesn't tell you to what contexts events were delivered, unless you add bookkeeping to every call into the VM.
I don't think we want to go down that road and I'm not sure if it would even be enough. Microtasks, for example, are mostly outside of our control - they're driven by V8 - so there would still be loopholes.
1. In reality you can't right now because
uv_run()is not re-entrant.Closing, there's nothing that can be done about it
If there is no reasonable way to fix this, then the documentation should have a notice about
timeoutnot beeing reliable in some cases (and about the shared microtask queue, perhaps).@ChALkeR oh, ok. saying that it's not reliable is not correct though, we should try to explain what it does and does not
64 remaining items
The issue itself is a
wontfixas noted before and was documented by 5e5a945.
Feel free to reopen if there is anything else to do here.Fwiw, I’m working on a fix for this, for the
Promisecase, so I’ll reopen this.(The
queueMicrotask()andnextTick()examples are not really fixable, but I wouldn’t consider them bugs or actual issues either, just misunderstandings about what those functions do.)Reacted by Denys Otrishko- added a commit that references this issue
on Jun 22, 2020 - added 2 commits that reference this issue
on Jul 14, 2020 - added a commit that references this issue
on Jul 27, 2026
The timeout property on many of the vm module's functions isn't completely foolproof. In the following example, some sandboxed code executed by vm.runInNewContext with a timeout of 5ms schedules an infinite loop to run after a promise resolves, and then synchronously executes an infinite loop. The synchronous infinite loop is killed by the timeout, but then the scheduled loop fires and never ends.
Some output:
I'm not really sure what a good solution for this would be, if it's possible, and whether the vm module intends to stand up to this sort of thing. At the very least I think vm's docs should have a note that the timeout property works by best-effort or something and isn't completely foolproof, so no one thinks it's safe to try to use it to sandbox user code in a guaranteed timely manner.