mqtt5 client event handler bindings leave open handles keeping node process running

[dtyau]

Describe the bug

The AWS MQTT5 client leaves open handles on its event handlers that are instantiated within the constructor. These open handles prevent NodeJS process from closing properly.

I was able to create a simple script that replicates the issue. It shows the same open handles that I've found when using why-is-node-running with our test framework.

The open handles are are from the AWS MQTT5 Client event handler bindings shown by why-is-node-running:

There are 6 handle(s) keeping the process running

# aws_mqtt5_client_on_stopped
<my workspace directory>/aws-iot-device-sdk-v2-repro/node_modules/aws-crt/dist/native/mqtt5.js:143 - this._super(binding_1.default.mqtt5_client_new(this, config, (client) => { Mqtt5Client._s_on_stopped(client); }, (client) => { Mqtt5Client._s_on_attempting_connect(client); }, (client, connack, settings) => { Mqtt5Client._s_on_connection_success(client, connack, settings); }, (client, errorCode, connack) => { Mqtt5Client._s_on_connection_failure(client, new error_1.CrtError(errorCode), connack); }, (client, errorCode, disconnect) => { Mqtt5Client._s_on_disconnection(client, new error_1.CrtError(errorCode), disconnect); }, (client, message) => { Mqtt5Client._s_on_message_received(client, message); }, config.clientBootstrap ? config.clientBootstrap.native_handle() : null, config.socketOptions ? config.socketOptions.native_handle() : null, config.tlsCtx ? config.tlsCtx.native_handle() : null, config.httpProxyOptions ? config.httpProxyOptions.create_native_handle() : null));
<my workspace directory>/aws-iot-device-sdk-v2-repro/test.js:37                                    - const client = new mqtt5.Mqtt5Client(mqttConfig);

# aws_mqtt5_client_on_attempting_connect
<my workspace directory>/aws-iot-device-sdk-v2-repro/node_modules/aws-crt/dist/native/mqtt5.js:143 - this._super(binding_1.default.mqtt5_client_new(this, config, (client) => { Mqtt5Client._s_on_stopped(client); }, (client) => { Mqtt5Client._s_on_attempting_connect(client); }, (client, connack, settings) => { Mqtt5Client._s_on_connection_success(client, connack, settings); }, (client, errorCode, connack) => { Mqtt5Client._s_on_connection_failure(client, new error_1.CrtError(errorCode), connack); }, (client, errorCode, disconnect) => { Mqtt5Client._s_on_disconnection(client, new error_1.CrtError(errorCode), disconnect); }, (client, message) => { Mqtt5Client._s_on_message_received(client, message); }, config.clientBootstrap ? config.clientBootstrap.native_handle() : null, config.socketOptions ? config.socketOptions.native_handle() : null, config.tlsCtx ? config.tlsCtx.native_handle() : null, config.httpProxyOptions ? config.httpProxyOptions.create_native_handle() : null));
<my workspace directory>/aws-iot-device-sdk-v2-repro/test.js:37                                    - const client = new mqtt5.Mqtt5Client(mqttConfig);

# aws_mqtt5_client_on_connection_success
<my workspace directory>/aws-iot-device-sdk-v2-repro/node_modules/aws-crt/dist/native/mqtt5.js:143 - this._super(binding_1.default.mqtt5_client_new(this, config, (client) => { Mqtt5Client._s_on_stopped(client); }, (client) => { Mqtt5Client._s_on_attempting_connect(client); }, (client, connack, settings) => { Mqtt5Client._s_on_connection_success(client, connack, settings); }, (client, errorCode, connack) => { Mqtt5Client._s_on_connection_failure(client, new error_1.CrtError(errorCode), connack); }, (client, errorCode, disconnect) => { Mqtt5Client._s_on_disconnection(client, new error_1.CrtError(errorCode), disconnect); }, (client, message) => { Mqtt5Client._s_on_message_received(client, message); }, config.clientBootstrap ? config.clientBootstrap.native_handle() : null, config.socketOptions ? config.socketOptions.native_handle() : null, config.tlsCtx ? config.tlsCtx.native_handle() : null, config.httpProxyOptions ? config.httpProxyOptions.create_native_handle() : null));
<my workspace directory>/aws-iot-device-sdk-v2-repro/test.js:37                                    - const client = new mqtt5.Mqtt5Client(mqttConfig);

# aws_mqtt5_client_on_connection_failure
<my workspace directory>/aws-iot-device-sdk-v2-repro/node_modules/aws-crt/dist/native/mqtt5.js:143 - this._super(binding_1.default.mqtt5_client_new(this, config, (client) => { Mqtt5Client._s_on_stopped(client); }, (client) => { Mqtt5Client._s_on_attempting_connect(client); }, (client, connack, settings) => { Mqtt5Client._s_on_connection_success(client, connack, settings); }, (client, errorCode, connack) => { Mqtt5Client._s_on_connection_failure(client, new error_1.CrtError(errorCode), connack); }, (client, errorCode, disconnect) => { Mqtt5Client._s_on_disconnection(client, new error_1.CrtError(errorCode), disconnect); }, (client, message) => { Mqtt5Client._s_on_message_received(client, message); }, config.clientBootstrap ? config.clientBootstrap.native_handle() : null, config.socketOptions ? config.socketOptions.native_handle() : null, config.tlsCtx ? config.tlsCtx.native_handle() : null, config.httpProxyOptions ? config.httpProxyOptions.create_native_handle() : null));
<my workspace directory>/aws-iot-device-sdk-v2-repro/test.js:37                                    - const client = new mqtt5.Mqtt5Client(mqttConfig);

# aws_mqtt5_client_on_disconnection
<my workspace directory>/aws-iot-device-sdk-v2-repro/node_modules/aws-crt/dist/native/mqtt5.js:143 - this._super(binding_1.default.mqtt5_client_new(this, config, (client) => { Mqtt5Client._s_on_stopped(client); }, (client) => { Mqtt5Client._s_on_attempting_connect(client); }, (client, connack, settings) => { Mqtt5Client._s_on_connection_success(client, connack, settings); }, (client, errorCode, connack) => { Mqtt5Client._s_on_connection_failure(client, new error_1.CrtError(errorCode), connack); }, (client, errorCode, disconnect) => { Mqtt5Client._s_on_disconnection(client, new error_1.CrtError(errorCode), disconnect); }, (client, message) => { Mqtt5Client._s_on_message_received(client, message); }, config.clientBootstrap ? config.clientBootstrap.native_handle() : null, config.socketOptions ? config.socketOptions.native_handle() : null, config.tlsCtx ? config.tlsCtx.native_handle() : null, config.httpProxyOptions ? config.httpProxyOptions.create_native_handle() : null));
<my workspace directory>/aws-iot-device-sdk-v2-repro/test.js:37                                    - const client = new mqtt5.Mqtt5Client(mqttConfig);

# aws_mqtt5_client_on_message_received
<my workspace directory>/aws-iot-device-sdk-v2-repro/node_modules/aws-crt/dist/native/mqtt5.js:143 - this._super(binding_1.default.mqtt5_client_new(this, config, (client) => { Mqtt5Client._s_on_stopped(client); }, (client) => { Mqtt5Client._s_on_attempting_connect(client); }, (client, connack, settings) => { Mqtt5Client._s_on_connection_success(client, connack, settings); }, (client, errorCode, connack) => { Mqtt5Client._s_on_connection_failure(client, new error_1.CrtError(errorCode), connack); }, (client, errorCode, disconnect) => { Mqtt5Client._s_on_disconnection(client, new error_1.CrtError(errorCode), disconnect); }, (client, message) => { Mqtt5Client._s_on_message_received(client, message); }, config.clientBootstrap ? config.clientBootstrap.native_handle() : null, config.socketOptions ? config.socketOptions.native_handle() : null, config.tlsCtx ? config.tlsCtx.native_handle() : null, config.httpProxyOptions ? config.httpProxyOptions.create_native_handle() : null));
<my workspace directory>/aws-iot-device-sdk-v2-repro/test.js:37                                    - const client = new mqtt5.Mqtt5Client(mqttConfig);
^C

If you have any idea of how to resolve/avoid this issue, please let me know.

Regression Issue

• Select this option if this issue appears to be a regression.

Expected Behavior

I expect the AWS MQTT5 client event handlers to be cleaned up successfully allowing the NodeJS process to close successfully.

Current Behavior

The AWS MQTT5 client leave open handles for the event handlers which prevents NodeJS process from closing and causing it to hang.

Reproduction Steps

Here is a simple package file and NodeJS script file to replicate the issue, my coworker was able to get the same console output with these. However, you'll need your own AWS IoT endpoint, private key pem file and client certificate file.

package.json

{
  "dependencies": {
    "aws-iot-device-sdk-v2": "^1.21.1",
    "why-is-node-running": "^2.3.0"
  }
}

test.js script

const { mqtt5, iot } = require('aws-iot-device-sdk-v2');
const whyIsNodeRunning = require('why-is-node-running');

const awsIotEndpoint = '<your endpoint>';

const builder = iot.AwsIotMqtt5ClientConfigBuilder.newDirectMqttBuilderWithMtlsFromPath(
    awsIotEndpoint,
    './clientCertificate.pem',
    './awsIoTPrivateKey.pem'
);

builder.withConnectProperties();

const mqttConfig = builder.build();

const client = new mqtt5.Mqtt5Client(mqttConfig);

const main = async () => {
    whyIsNodeRunning();
};

main();

And just run npm install and node ./test.js.

Possible Solution

No response

Additional Information/Context

No response

aws-crt-nodejs version used

1.21.8

nodejs version used

20.18.1

Operating System and version

Ubuntu 22.04.5 LTS

[bretambrose]

Have you tried https://awslabs.github.io/aws-crt-nodejs/node/classes/mqtt5.Mqtt5Client.html#close ?

[dtyau]

Yes, we've implemented it properly with initialize(), stop(), close() and even with the disconnect packet for stop(). Everything that was async was awaited properly.

The reproduction provided is a distilled version to provide the least amount of code.

[bretambrose]

Calling close makes the client eligible for garbage collection. The bindings are destroyed as part of garbage collection, but we cannot force node to garbage collect anything.

[dtyau]

I've tried to force node to garbage collect with --expose-gc and global.gc() (https://stackoverflow.com/a/30654451) but it didn't change the result. There are still open handles after calling close() and global.gc().

[bretambrose]

The only way I've found to "reliably" trigger actual GC is to apply memory pressure (large local allocations) while invoking the gc (which might not actually be necessary).

[bretambrose]

Still thinking about this. There may be something we can do but I'll need to convince others that the investigation time is worthwhile. If we get something working, it may also be applicable to the longstanding #291

[dtyau]

Thanks for the quick responses. We've managed to workaround this issue by using MQTT.js. If this is ever investigated further, that might make a good reference for comparison.

We can close this issue if desired or leave it open as a record of the issue.