cloudant / nodejs-cloudant

Cloudant Node.js client library
Apache License 2.0
255 stars 90 forks source link

Change timeout handling in TokenManager #440

Closed vmatyus closed 3 years ago

vmatyus commented 3 years ago

Checklist

Description

According to the snapshot created by Eric, clearly can be seen there is a setTimeout that binds data to the native context.

Native objects are everything else which is not in the JavaScript heap.

Screenshot 2021-04-01 at 15 06 45

This code part refers to this:

setTimeout(this._autoRenew.bind(this, defaultMaxAgeSecs), delayMSecs).unref()

Here is small code example that produce the same memory trace:

var heapdump = require('heapdump');

function sleep(seconds){
    var waitUntil = new Date().getTime() + seconds*1000;
    while(new Date().getTime() < waitUntil) true;
}

function print() {
    console.log("Hello")
}

setTimeout(print, 10).unref();

sleep(2)

heapdump.writeSnapshot('./timeout-unref.heapsnapshot');

This produce the same memory trace: Screenshot 2021-04-01 at 15 35 15

Approach

Extending the sample code with a timer clear looks like this:

var heapdump = require('heapdump');

function sleep(seconds){
    var waitUntil = new Date().getTime() + seconds*1000;
    while(new Date().getTime() < waitUntil) true;
}

function print() {
    console.log("Hello")
}

myTimer = setTimeout(print, 10);

sleep(1)

clearTimeout(myTimer)
myTimer = setTimeout(print, 10);

sleep(1)

clearTimeout(myTimer)

heapdump.writeSnapshot('./timeout-var.heapsnapshot');

And this code memory trace looks this way: Screenshot 2021-04-01 at 15 52 43

This means that the referenced object will be placed into the global not into the system/context scope.

Hopefully this change will allow for the native objects to get cleared by GC.

Schema & API Changes

Security and Privacy

Testing

Travis has some pending problem, but do not know why.

Monitoring and Logging

vmatyus commented 3 years ago

This correction is not enough to remove the memory leak.