Skip to content
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

serialport memory leak on raspberry pi 3 #1477

Closed
fdedraco opened this issue Feb 13, 2018 · 13 comments
Closed

serialport memory leak on raspberry pi 3 #1477

fdedraco opened this issue Feb 13, 2018 · 13 comments

Comments

@fdedraco
Copy link

fdedraco commented Feb 13, 2018

  • SerialPort Version: 6.0.4 & 6.1.4

  • NodeJS Version: 8.9.x & 9.5.0

  • Operating System and Hardware Platform: raspbian strectch lite on raspberry pi 3

  • Have you checked the right version of the api docs?: yes, it kinda works as intended

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

  • Are you using Electron and have you checked the Electron Docs?: no

Summary of Problem

i was very happy that my project is almost complete but when i test the project on raspberry pi it crashes after running for a while, it crashes, i suspect it's some kind of memory leak, since the node memory usage is increased overtime, console error at bottom of the post

Steps and Code to Reproduce the Issue

i make a server that can change used serial port when needed

re-config script:

      console.log( data ); 
      // typically -> data = {serport:"/dev/ttyAMA0",serbaud: "38400"}
      // remove event listeners
      myport.removeAllListeners( 'data' )
      myport.removeAllListeners( 'open' )
      myport.removeAllListeners( 'close' )
      myport.removeAllListeners( 'error' )
      console.log( "attempt to close port" );
      if ( myport.isOpen ) {
        myport.close()
      }
      console.log( "connecting to port" + data.serport );
      myport = new serialport( data.serport, {
        baudRate: parseInt( data.serbaud )
      } );
      console.log( "should be closed" );
      parser = new Readline();
      myport.pipe( parser );
      parser.on( 'data', processdata );
      myport.on( 'open', showPortOpen );
      myport.on( 'close', showPortClose );
      myport.on( 'error', showError );

event handlers:

function processdata( data ) {
  lastvalue = data
  if ( run === 1 ) {
    raw.push( data );
    if ( raw.length > 1000 ) {
      raw.shift()
      // console.log( raw.length+"!"+ data);
    } else {
      // console.log( raw.length+">"+ data);
    }
  }
}

function showPortClose() {
  console.log( 'port closed.' );
}

function showError( error ) {
  console.log( 'Serial port error: ' + error );
}

didn't crash when the said function didn't called
edit: no data is received yet from serial

console error:

<--- Last few GCs --->

[1181:0x1bcb370]   650266 ms: Mark-sweep 703.5 (729.0) -> 703.4 (729.0) MB, 6409.4 / 0.1 ms  (+ 0.0 ms in 0 steps since start of marking, biggest step 0.0 ms, walltime since start of marking 6409 ms) last resort GC in old space requested
[1181:0x1bcb370]   656674 ms: Mark-sweep 703.4 (729.0) -> 703.4 (729.0) MB, 6407.8 / 0.1 ms  last resort GC in old space requested


<--- JS stacktrace --->

==== JS stack trace =========================================

Security context: 0x3b71632d <JSObject>
    2: oncomplete(aka wrapper) [fs.js:656] [bytecode=0x71363f8d offset=0](this=0x3b3fe029 <FSReqWrap map = 0x5b6d7c6d>,err=0x4c284101 <null>,bytesRead=0)

==== Details ================================================

[2]: oncomplete(aka wrapper) [fs.js:656] [bytecode=0x71363f8d offset=0](this=0x3b3fe029 <FSReqWrap map = 0x5b6d7c6d>,err=0x4c284101 <null>,bytesRead=0) {
  // expression stack (top to ...

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory
@fdedraco
Copy link
Author

update, on the latest version, it didn't crash, but still using big amount of memory (700+ MB) for some reason, compared to windows counterpart that only use 25MB of memory

@fdedraco
Copy link
Author

nodejs 6.x installed, everything seems fine

@reconbot
Copy link
Member

reconbot commented Feb 13, 2018 via email

@fdedraco
Copy link
Author

umm the other way, it leaks on v8 and v9, running fine on v6
and just in raspi, my desktop version is doing fine,
would gcc version affect compilation?

@reconbot
Copy link
Member

gcc might make it fail, I'm not sure it would cause a memory leak

@npeditto
Copy link

npeditto commented Mar 6, 2018

Hi... I have the same problem on Raspberry Pi 3 with:

  • SerialPort Version: 6.0.4
  • NodeJS Version: 7.10.x
  • RASPBIAN STRETCH WITH DESKTOP

both with "data" and "readable" event listeners; it uses 700MB of memory at least.

Thanks in advance!

Nicola

@shodan8192
Copy link

shodan8192 commented May 25, 2018

Hello, the same on :

  • Raspbian Stretch Lite on Raspberry Pi 2B v1.2
  • Ubuntu 17.10 on desktop PC

tested with :

  • NodeJS v8.11.2, v6.14.2
  • SerialPort v6.0.4, v6.2.0

I'm using SerialPort as dependency of modbus-serial, in program running on RPi2, which collecting data from few PLCs. Memory consumption initially is about 50MB, growing to 220MB after 24h, and to 310MB after next 12h. RPi is not fast, so when mem utilization is high, GC runs take some time - it causing many timeouts when reading modbus.

Additionally, I found closed #1187 issue, and example code provided in them (after remove port.write - only open and close port cycle) quickly causing memory leak on my Ubuntu PC.

after 1 sec :
rss       : 64.4 MB
heapTotal : 26.3 MB
heapUsed  : 10.5 MB

after 1 min :
rss       : 382.0 MB
heapTotal : 205.8 MB
heapUsed  : 135.1 MB

after 5 min :
rss       : 896.5 MB
heapTotal : 544.8 MB
heapUsed  : 390.7 MB

When this code is running, forcing GC from DevTools inspector drops memory usage only by few MBs.

EDIT :
Mentioned code, modified that port is opened once, and then writing data in infinite loop have stable memory usage

after 30 min :
rss       : 63.9 MB
heapTotal : 39.3 MB
heapUsed  : 9.5 MB

EDIT 2 :
I have also checked reading stream using 'data' event with simple code - it's ok

after 30 min :
rss       : 102.5 MB
heapTotal : 38.8 MB
heapUsed  : 7.3 MB
received  : 3966.1 MB

so definitely problem is in port.open, port.close or both of them.

@shodan8192
Copy link

shodan8192 commented May 30, 2018

I found the main cause of memory "leak" (there is no leak in strict sense, that's why valgrind doesn't find it) - native Poller object is created each time we create SerialPort - but never get disposed.

If I understand correctly, it should happen when on JS side null is assigned to Poller object, then GC run and call WeakCallback (attached during wrap Poller object), but v8 doc state : "There is no guarantee as to when or even if the callback is invoked".

Quick & dirty solution I use in my production program is to add delete obj; in poller.cpp

NAN_METHOD(Poller::stop)
{
  Poller *obj = Nan::ObjectWrap::Unwrap<Poller>(info.Holder());
  obj->stop();
  delete obj; // <-- added
}

and remove assertion in node_modules/nan/nan_object_wrap.h

  virtual ~ObjectWrap()
  {
    if (persistent().IsEmpty())
    {
      return;
    }

   // assert(persistent().IsNearDeath()); // <-- commented out
    persistent().ClearWeak();
    persistent().Reset();
  }

that's do the trick, and memory consumption does not rise.

For sure it isn't valid or elegant way to do this - I have virtually no experience in C++, so my question is how and where manually and properly dispose Poller object ?

@reconbot
Copy link
Member

We can't modify Nan but I'm sure there is another way! I'm on leave this month (taking time off from coding in general) but I'm very happy to see this investigation start to bear fruit!

@shodan8192
Copy link

I think I have real solution without have to modify Nan. The key was to call obj->persistent().Reset(); before delete obj;

I have added new method "destroy" to Poller, which is intended to be called from JS after poller.stop() and before assigning null to poller object. For all necessary changes, I made a patch
poller.patch.gz against commit d829ada, for You and anyone who is interested, to be reviewed and maybe to merge if it's ok.

@reconbot
Copy link
Member

reconbot commented Jun 1, 2018 via email

@shodan8192
Copy link

Pull request been has opened. I can also mention, that my telemetry program, after patch of serialport have two weeks of uptime now, and memory usage is stable.

@reconbot
Copy link
Member

I merged #1572 (your fix) and I'll open another issue about finding a way to handle the case where destroy doesn't get called because the object went out of scope. I'll keep this issue open until the patch has been released.

@reconbot reconbot added bug and removed support labels Jun 24, 2018
@reconbot reconbot self-assigned this Jun 24, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Mar 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

4 participants