[dave-365]

[dave-365]

Problem Background

During application hot reload, pending AsyncProgressWorker operations can cause Node.js environment teardown issues, potentially resulting in event-loop hangs or application crashes.

This is particularly relevant to developers using node-addon-api in applications with hot-reload workflows.

Related discussion: [Issue #1109](#1109)

Proposed Solution

This PR introduces environment cleanup handling for AsyncProgressWorkerBase to ensure that pending asynchronous operations are properly handled during environment teardown.

The intention is to prevent event-loop hangs and improve application stability during hot reload.

Impact

This fix may benefit developers using AsyncProgressWorker in applications that frequently reload or recreate their Node.js environments.

Review Request

Could the maintainers please review the proposed cleanup approach and provide feedback on any lifecycle or thread-safety concerns?

[KevinEady]

Hi @dave-365 ,

Is there any way to include a test in this PR? We have some tests already that check for various behavior in terminating environments, e.g. https://github.com/nodejs/node-addon-api/blob/main/test/error_terminating_environment.js . Maybe you can follow this pattern?

[dave-365]

Hi @dave-365 ,

Is there any way to include a test in this PR? We have some tests already that check for various behavior in terminating environments, e.g. https://github.com/nodejs/node-addon-api/blob/main/test/error_terminating_environment.js . Maybe you can follow this pattern?

@KevinEady Okay I will include test in this PR.

[dave-365]

Hi @dave-365 ,

Is there any way to include a test in this PR? We have some tests already that check for various behavior in terminating environments, e.g. https://github.com/nodejs/node-addon-api/blob/main/test/error_terminating_environment.js . Maybe you can follow this pattern?

Hi @KevinEady

I have addressed the review feedback and added automated tests following the pattern in test/error_terminating_environment.js:

1. Automated Test Coverage: Added test/async_progress_worker_terminating_environment.js testing both:
   • Environment termination while the worker thread is actively executing and sending progress.
   • Normal completion prior to environment teardown (verifying cleanup hook removal and no use-after-free).
   • Both scenarios are tested against C++ exceptions (binding.node) and no-exceptions (binding_noexcept.node) configurations.
2. Lifecycle & UAF Prevention:
   • In ~AsyncProgressWorkerBase(), _cleanup_hook.Remove(Env()) safely removes the hook when the worker completes normally.
   • If OnEnvironmentTeardown() runs, _cleanup_hook is reset to empty to avoid duplicate removal.
   • If the environment is terminating, status is marked napi_cancelled so JavaScript callbacks (OnOK/OnError) are skipped during teardown, cleanly routing to destruction.
3. Linting and Full Test Suite: All tests pass (node test), and formatting complies with clang-format and eslint.