memory leak when continuously writing?

[jsyzqt]

• SerialPort Version: 6.0.4 & 6.0.5 & 6.1.0

• NodeJS Version: v8.9.0

• Operating System and Hardware Platform: Windows 10 x64 [10.0.16299.248] & Arduino Due

• Have you checked the right version of the api docs?:

• Are you having trouble installing and you checked the Installation Special Cases docs?

• Are you using Electron and have you checked the Electron Docs?:
  First used in Electron, and then test in a pure node environment after encountering the problem.

Summary of Problem

I experienced that when continuously writing data to the serial port, large numbers of 'Buffer' objects were created, which can be observed in DevTools using 'Take snapshot'. If the transmit speed was accelerated, sometimes the programs will occupy hundreds of MBs of memory in a daytime. I have tried the versions of 6.0.4 & 6.0.5 & 6.1.0.

Steps and Code to Reproduce the Issue

I simply use the following test codes.

var port = new SerialPort("COM20", {
    baudRate: defaultBaudRate
});
setTimeout(()=>{
    setInterval(() => {
        port.write(Buffer.from('##0115aC8**', 'ascii'));
    }, 300);

    port.on('data', function(data){
        console.log(data.toString('ascii'));
    });
}, 400);

[reconbot]

You're not waiting for the writes to finish so they probably just queue.

…

On Tue, Feb 20, 2018, 11:45 AM jsyzqt ***@***.***> wrote: - *SerialPort Version*: 6.0.4 & 6.0.5 & 6.1.0 - *NodeJS Version*: v8.9.0 - *Operating System* and *Hardware Platform*: Windows 10 x64 [10.0.16299.248] - Have you checked the right *version* of the *api docs?*: - Are you having trouble installing and you checked the *Installation Special Cases* docs? - Are you using *Electron* and have you checked the *Electron Docs*?: First used in Electron, and then test in a pure node environment after encountering the problem. Summary of Problem I experienced that when continuously writing data to the serial port, large numbers of 'Buffer' objects were created, which can be observed in DevTools using 'Take snapshot'. If the transmit speed was accelerated, sometimes the programs will occupy hundreds of MBs of memory in a daytime. I have tried the versions of 6.0.4 & 6.0.5 & 6.1.0. Steps and Code to Reproduce the Issue I simply use the following test codes. var port = new SerialPort("COM20", { baudRate: defaultBaudRate }); setTimeout(()=>{ setInterval(() => { port.write(Buffer.from('##0115aC8**', 'ascii')); }, 300); port.on('data', function(data){ console.log(data.toString('ascii')); }); }, 400); — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1496>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABlbjPjPnI-iibIju-WIzgyhKh9BxwWks5tWvazgaJpZM4SMRM_> .

[jsyzqt]

@reconbot There are enough time intervals between sending intructions. Actually, the response data is correct and in time. My arduino reply instructions every 50ms, so I think the time interval should be enough.

I means i have observed the tx data(to the arduino) and the rx data (from the arduino) in 300ms, actually it takes just a few milliseconds.

[reconbot]

If you could maybe setTimeout after the write just to be sure that would help you debug.

[jsyzqt]

@reconbot Recently, I have tried the following code:

const promisify = require('util').promisify;
const SerialPort = require('serialport');

var port = new SerialPort("COM20", {
    baudRate: 115200
});

port.on('data', function(data){
    console.log(data.toString('ascii'));
});

(function test(){
    promisify(wAd)().then(()=>{
        test();
    });
})();

function wAd(callback){
    port.write(Buffer.from('##0115aC8**', 'ascii'));
    port.drain((err)=>{
        setTimeout(()=>{
            callback();
        }, 1000);
    });
}

However, the problem is still same as what I have previously said. The 'buffer' objects are accumulating after starting the program. But, I find it works well on Ubuntu 16.04 and the memory usage is stable.

[benpurdy]

I have seen this same problem, also using Windows. The "rss" memory reported from node will steadily climb as the serial port is used (I need to write to the port around 40 times per second the entire time the app is running, and the app needs to run for many days).

I have been able to fix the issue by adding a single line to serialport_win.cpp

Please note that I'm extremely unfamiliar with native code binding in node but from what I can understand in the documentation for nan::Persistent, it seems like V8 might be keeping the underlying memory around even though the baton has been deleted since baton->buffer.Reset(buffer) is called but baton->buffer.Empty() or baton->buffer.Reset() aren't called later.

Anyhow, adding baton.buffer.Reset() just before delete baton; on line #387 in serialport_win.cpp appears to resolve this problem on my machine.

[reconbot]

That's interesting. It's a different line in master the AFTER_WRITE() right? This is the only place we keep a buffer any longer. (We used to do this for all systems.) I'm not familiar enough to know if we're freeing it correctly. I think reset() is probably setting it to be 0 length which is freeing most of it but probably not the object itself.

@indutny do you know off the top of your head?

[benpurdy]

Yes, that's correct about the different line number. As I said I'm not familiar with the node native code binding system (nor is C++ a language I use very often) but I'm basing my guess on the docs for nan::Persistent

https://github.com/nodejs/nan/blob/master/doc/persistent.md

"An object reference that is independent of any HandleScope is a persistent reference. Where a Local handle only lives as long as the HandleScope in which it was allocated, a Persistent handle remains valid until it is explicitly disposed."

Then later it says:

"Existing handles can be disposed using an argument-less Nan::PersistentBase::Reset()."

[reconbot]

That is exactly what we want to do, we're done with the write and it should let the buffer go. Thank you for finding this. Do you mind submitting a patch?

…

--- Francis Gulotta wizard@roborooter.com

On Mon, Apr 9, 2018 at 7:18 PM, benpurdy ***@***.***> wrote: Yes, that's correct about the different line number. As I said I'm not familiar with the node native code binding system (nor is C++ a language I use very often) but I'm basing my guess on the docs for nan::Persistent https://github.com/nodejs/nan/blob/master/doc/persistent.md "An object reference that is independent of any HandleScope is a persistent reference. Where a Local handle only lives as long as the HandleScope in which it was allocated, *a Persistent handle remains valid until it is explicitly disposed.*" Then later it says: "Existing handles can be disposed using an argument-less Nan::PersistentBase::Reset()." — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1496 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABlbh1QqCK1R3119LP02yLSKwbBSRspks5tm-xPgaJpZM4SMRM_> .

[benpurdy]

Oh, I was just looking at the source again, would it be better to dispose of the buffer in the WriteIOCompletion function?

I don't know if this is a problem or not but my previous solution will release the buffer as soon as the write is queued (via EIO_AfterWrite) rather than when the write operation is actually finished..

[reconbot]

I'll double check but I don't think we call after write until it's completed.

…

On Mon, Apr 9, 2018, 7:38 PM benpurdy ***@***.***> wrote: Oh, I was just looking at the source again, would it be better to dispose of the buffer in the WriteIOCompletion <https://github.com/node-serialport/node-serialport/blob/9dbccbc755e622b1c3d625625af4c627cc98d432/src/serialport_win.cpp#L345> function? I don't know if this is a problem or not but my previous solution will release the buffer as soon as the write is queued (via EIO_AfterWrite) rather than when the write operation is actually *finished*.. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1496 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABlbhKNjW9vdzhj3dNV9L0j42mfgXuGks5tm_D9gaJpZM4SMRM_> .