Node process exits regardless of open socket

[phra]

hi, i am doing some tests with your library but my node process always quits after around 10 seconds.

source code:

var raw = require ("raw-socket");

var socket = raw.createSocket ({protocol: raw.Protocol.ICMP});

socket.on ("message", function (buffer, source) {
    console.log ("received " + buffer.length + " bytes from " + source);
});

i would expect it to remain alive, as stated in the readme, but for some reason, it just exists after a short time.

tested with node v12.20.0

[stephenwvickers]

This works for me...

[phra]

thanks for the reply. have you tried with node v12.20.0 ?

[stephenwvickers]

I thought I did, but it looks like no. I will try again with v12.

[stephenwvickers]

Sure enough, using 12.22.1, after about 8 seconds it exits:

[stephen@dev-centos7 tmp]$ sudo node app.js
received 84 bytes from 8.8.8.8
received 84 bytes from 8.8.8.8
received 84 bytes from 8.8.8.8
received 84 bytes from 8.8.8.8
received 84 bytes from 8.8.8.8
received 84 bytes from 8.8.8.8
received 84 bytes from 8.8.8.8
received 84 bytes from 8.8.8.8
[stephen@dev-centos7 tmp]$

I will take a look.

[algj]

I have tried v14.15.4 on my server, it quits in a second, but for some reason it does not in REPL and it works as expected. Very weird.

Tested on local machine with v15.5.1 and it works.

I upgraded to v15.5.1 on my server and had tons of errors when installing with npm i raw-socket related to compiling (I can upload those errors if needed).

I downgraded node.js to v10.23.1 on my server and it seems to work without any problem.

[phra]

it could be a regression in node itself. maybe is it worth it to open an issue on https://github.com/nodejs/node

[algj]

I reproduced the problem on machine.

I tested 12 ICMP requests and instantly sent back the reply, it would just close without any error.
I also tested just receiving 25 ICMP requests, it would close down when 25th request was received and processed in node.js.
After printing out the 25th request data, node.js would exit instantly after it.

Using Linux, node.js v15.5.1

[algj]

There's something very weird going on:
Using icmp library (which uses node-raw-socket) it reads exactly 68 requests:

const icmp = require('icmp')
let i = 0;
icmp.listen(()=>console.log(++i));

With my other code I could read exactly 79 requests, not more and not less.

I tested it like 5 times each, how quickly pings come doesn't make any difference when it would close.

It seems over REPL it works correctly (sometimes):
Works very well (tested 4000 requests):

$ node
const icmp = require('icmp'); let i = 0; icmp.listen(()=>console.log(++i));

$ sudo ping 127.0.0.1 -f

But this seems to be not working (only 80 received requests):

$ node
require('./index.js');

/* index.js
const icmp = require('icmp'); let i = 0; icmp.listen(()=>console.log(++i));
*/

If you put setInterval in your script, it won't exit, but it will stop receiving requests.

I tried NOT outputting anything, it did help quite a bit (over 300 requests without closing) but it did not fix the problem.

It might be some V8 optimization...

[algj]

This is a possible memory leak if opened a lot of times, but I guess no one's doing that?
At least it works.

    // quick fix for #70 issue
    var gcFix = setInterval(()=>this.wrap, 2147483647).unref();
    this.wrap.on('close', ()=>{clearInterval(gcFix));

This is one horrifying fix which at least works.

[phra]

i would suggest writing a minimized example that shows different behaviors across some node versions and then opening an issue on https://github.com/nodejs/node to report the potential regression.

[algj]

i would suggest writing a minimized example that shows different behaviors across some node versions and then opening an issue on https://github.com/nodejs/node to report the potential regression.

My best guess is that it's the raw.cc (raw.node) which has a problem.

I guess other way to solve this problem is putting it in an array/object.

This fix is better than having a non-working code.

[stephenwvickers]

Hi @algiuxass

My best guess is that it's the raw.cc (raw.node) which has a problem.

What do you think specifically is wrong with it?

I might get around to testing this tonight. I think the patch you provided is not a good fix, I'd rather understand the problem a little more.

The problem doesn't manifest itself to me in node 12.20.1, can you see if that's the same for you?

[algj]

Something gets unloaded by garbage collector. Not sure what exactly, but I know for sure this.wrap gets unloaded.

[stephenwvickers]

@algiuxass How do you know something gets unloaded by the garbage collector?

[algj]

1. Using debugger/REPL it will work as long as this.wrap is accessible from console.
2. In REPL if you use require('./index.js') it won't work, I assume because those variables are inaccessible so they get garbage collected.
3. Depending on memory usage, like how script is written, you may get less or more requests before closing, those requests are exactly the same count, unless I change the script.

[stephenwvickers]

1. Using debugger/REPL it will work as long as this.wrap is accessible from console.

Probably the debugger/REPL itself is keeping the node program alive, so the problem probably still exists here.

2. In REPL if you use require('./index.js') it won't work, I assume because those variables are inaccessible so they get garbage collected.

What doesn't what specifically? Do you have code and output?

[algj]

@stephenwvickers if I put setInterval inside anywhere, it won't close but it may stop working. Check my longest comment above. You will see. Also I've edited message above.

[algj]

Basically if I would use this.wrap anywhere, like in an interval, it will continue working as long as it's used somewhere. REPL keeps this running because you can access this.wrap from console.

[stephenwvickers]

That's the same as the REPL. I suppose what I am getting at is that it could be anything, yes it could be something like you describe, but it could also be something in libuv too, it's just not possible to know until more debugging is done. At this point I would not say it is anything specifically.

[stephenwvickers]

And @algiuxass just to be clear, I am not trying to be difficult, I am just trying to make sure I understand what each of the comments mean.

[algj]

let exec = require('child_process').exec;

// for pinging 1000 times very quickly
exec("ping 127.0.0.1 -c 1000 -f");

// to make garbage collector do its thing
setInterval(()=>{
    global.gc();
},10);

(function(){
    
    let raw = require('raw-socket');
    let socket = raw.createSocket({ protocol: raw.Protocol.ICMP });

    let index = 0;
    socket.on('message', ()=>{
        console.log("Getting data!",index++);
    });

    setTimeout(()=>{
        console.log("After this socket gets dereferenced.");
        socket;
    }, 2000);
    
})()

Proof that it's garbage collected.

...
Getting data! 110
Getting data! 111
Getting data! 112
Getting data! 113
Getting data! 114
Getting data! 115
After this socket gets dereferenced.
<no more console logs>

Edit: use sudo node --expose-gc

[algj]

I think it's possible with WeakRef to make this work but I did not succeed with it. Edit: Oh boy how wrong was I, it's used for a totally different thing.

I am not familiar with libuv, maybe this helps: void uv_ref(uv_handle_t* handle)

Sorry if I'm not clear or doing weird fixes, just trying to be helpful in any way I can.

[stephenwvickers]

So I wrote this module some years ago and the way we run the event loop for send and receive is a little odd, so I am thinking whether the best path is to actually re-write the send/receive internals to use the Nan::AsyncWorker interface. From the libraries interface it's the same socket.on("message"), so the change is internally.

I am wondering whether it's simply a result of the organic evolution in Node.js has caused a regression in the way we interact with Node.js and the libuv event loop.

Let me dig deeper, and see how difficult it would be.

[temaivanoff]

@stephenwvickers
Hi, there is something new ?

[algj]

Hi, there is something new ?

I assume he will not fix this bug anytime soon, for now you could use my fork which works but it has a hacky solution to avoid gc (you can check README what it does).

npm i https://github.com/algj/node-raw-socket/

[vendornltd]

Hi @fast0490f and @algj

I've not had any time to work on this yet. I will get around to it eventually, but I'm struggling to find time away from work at the moment.

Appologies!

Steve