-
Notifications
You must be signed in to change notification settings - Fork 28.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Infinite loop at shutdown #47748
Comments
I've just seen the issue without the call to wasm. Here is another backtrace.
I don't know if |
And another trace mentioning wasm but I don't know if it is relevant
|
Possibly related to #47452 |
can you push the code to try reproduce this issue ? |
I don't have a minimal repro.
|
possibly related to https://bugs.chromium.org/p/v8/issues/detail?id=14281 |
A colleague of mine analyzed one of these hanging processes with gdb. His analysis might be useful.
When modifying the return values of ThrowOnJavascriptExecution::IsAllowed(isolate) and DumpOnJavascriptExecution::IsAllowed(isolate) using gdb to return true, the process exits properly. We haven't been able to pinpoint exactly which piece of code is the culprit, as it happens in different node contexts. Hope this helps. |
@legendecas I saw your work in #45907, which adds |
I think this is something we want to fix. Essentially if there is anything left in the FinalizationRegistry to clean up, we should be able to call into JS to do so. The alternative is that we drain all FGs before shutting down. |
#47748 (comment) is an excellent summary of the problem. When node is going to free an Environment and disallows JavaScript execution, V8 still tries to re-enqueue a A script to reliably reproduce the problem: // Flags: --expose-gc
const reg = new FinalizationRegistry(() => {
console.log('This FR callback should never be called');
});
function register() {
// Create a temporary object in a new function scope to allow it to be GC-ed.
reg.register({});
}
process.on('exit', () => {
// This is the final chance to execute JavaScript.
console.log('exit');
register();
// Queue a FinalizationRegistryCleanupTask by a testing gc request.
gc();
}); I don't think this is a problem related to D8's https://bugs.chromium.org/p/v8/issues/detail?id=14281 because D8 didn't disallow JavaScript execution at its
I believe the callback of a FinalizationRegistry is not guaranteed to be called, and https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/FinalizationRegistry#notes_on_cleanup_callbacks explains it well. Particularly, when a worker_thread is requested to shut down from other threads, it should try to avoid calling into JavaScript again. I think a solution could be to avoid enqueueing a |
At the point of `FreeEnvironment` and onwards, no JavaScript execution associated with the Environment should be triggered. Avoid draining platform tasks that can trigger JavaScript execution in `FreeEnvironment`. The holder of `node::Environment` should immediately call `node::MultiIsolatePlatform::UnregisterIsolate` and `v8::Isolate::Dispose` to cancel pending foreground tasks and join concurrent tasks after the environment was freed. `NodePlatform` can properly handle the case in `RunForegroundTask` when an Isolate out-lives its associated `node::Environment`. PR-URL: #51290 Fixes: #47748 Fixes: #49344 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
At the point of `FreeEnvironment` and onwards, no JavaScript execution associated with the Environment should be triggered. Avoid draining platform tasks that can trigger JavaScript execution in `FreeEnvironment`. The holder of `node::Environment` should immediately call `node::MultiIsolatePlatform::UnregisterIsolate` and `v8::Isolate::Dispose` to cancel pending foreground tasks and join concurrent tasks after the environment was freed. `NodePlatform` can properly handle the case in `RunForegroundTask` when an Isolate out-lives its associated `node::Environment`. PR-URL: #51290 Fixes: #47748 Fixes: #49344 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
At the point of `FreeEnvironment` and onwards, no JavaScript execution associated with the Environment should be triggered. Avoid draining platform tasks that can trigger JavaScript execution in `FreeEnvironment`. The holder of `node::Environment` should immediately call `node::MultiIsolatePlatform::UnregisterIsolate` and `v8::Isolate::Dispose` to cancel pending foreground tasks and join concurrent tasks after the environment was freed. `NodePlatform` can properly handle the case in `RunForegroundTask` when an Isolate out-lives its associated `node::Environment`. PR-URL: nodejs#51290 Fixes: nodejs#47748 Fixes: nodejs#49344 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
At the point of `FreeEnvironment` and onwards, no JavaScript execution associated with the Environment should be triggered. Avoid draining platform tasks that can trigger JavaScript execution in `FreeEnvironment`. The holder of `node::Environment` should immediately call `node::MultiIsolatePlatform::UnregisterIsolate` and `v8::Isolate::Dispose` to cancel pending foreground tasks and join concurrent tasks after the environment was freed. `NodePlatform` can properly handle the case in `RunForegroundTask` when an Isolate out-lives its associated `node::Environment`. PR-URL: nodejs#51290 Fixes: nodejs#47748 Fixes: nodejs#49344 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
At the point of `FreeEnvironment` and onwards, no JavaScript execution associated with the Environment should be triggered. Avoid draining platform tasks that can trigger JavaScript execution in `FreeEnvironment`. The holder of `node::Environment` should immediately call `node::MultiIsolatePlatform::UnregisterIsolate` and `v8::Isolate::Dispose` to cancel pending foreground tasks and join concurrent tasks after the environment was freed. `NodePlatform` can properly handle the case in `RunForegroundTask` when an Isolate out-lives its associated `node::Environment`. PR-URL: #51290 Fixes: #47748 Fixes: #49344 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
At the point of `FreeEnvironment` and onwards, no JavaScript execution associated with the Environment should be triggered. Avoid draining platform tasks that can trigger JavaScript execution in `FreeEnvironment`. The holder of `node::Environment` should immediately call `node::MultiIsolatePlatform::UnregisterIsolate` and `v8::Isolate::Dispose` to cancel pending foreground tasks and join concurrent tasks after the environment was freed. `NodePlatform` can properly handle the case in `RunForegroundTask` when an Isolate out-lives its associated `node::Environment`. PR-URL: #51290 Fixes: #47748 Fixes: #49344 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
At the point of `FreeEnvironment` and onwards, no JavaScript execution associated with the Environment should be triggered. Avoid draining platform tasks that can trigger JavaScript execution in `FreeEnvironment`. The holder of `node::Environment` should immediately call `node::MultiIsolatePlatform::UnregisterIsolate` and `v8::Isolate::Dispose` to cancel pending foreground tasks and join concurrent tasks after the environment was freed. `NodePlatform` can properly handle the case in `RunForegroundTask` when an Isolate out-lives its associated `node::Environment`. PR-URL: #51290 Fixes: #47748 Fixes: #49344 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Version
v16.18.1, v18.16.0 and v20.0.0
Platform
Linux 5.15.0-41-generic #44-Ubuntu SMP x86_64 x86_64 x86_64 GNU/Linux
Subsystem
No response
What steps will reproduce the bug?
I don't have an easy way to reproduce the issue.
How often does it reproduce? Is there a required condition?
It triggers once every 10 runs maybe, it triggers much more often if I set break breakpoints (e.g. node::PerIsolatePlatformData::FlushForegroundTasksInternal)
What is the expected behavior? Why is that the expected behavior?
I expect the process to terminate successfully.
What do you see instead?
The process hangs at 100% cpu after having done its normal job.
Attaching gdb gives me traces like
and
The process never return from
node::NodePlatform::DrainTasks
Additional information
Based on some name seen in the backtrace, the following information might be useful.
My process use
FinalizationRegistry
with a finalization function calling into wasm. Concretely, I've a mapping between some js value and some wasm allocations, and have finalisers on some js value that free the memory on the wasm side (callingfree
).I'm currently unable to reproduce the issue when removing the call to (wasm)
free
in the finaliser.The text was updated successfully, but these errors were encountered: