node-ffi-napi / ref-napi

Turn Buffer instances into "pointers"
MIT License
123 stars 67 forks source link

Memory leak on a simple use case #59

Closed toverux closed 3 years ago

toverux commented 3 years ago

Hi, Sorry for the title, I didn't really know how to describe the problem.

I'm integrating a .so library compiled with CGO's -builmode=c-shared. I'm trying to build a first example to check I'm able to do it properly. I'll call Go from Node and pass buffers of protobuf messages back and forth.

It's a PoC so the code is not very clean, my apologies.

The Go code looks like this:

/*
#include <stdlib.h>

typedef struct { void *data; int len; } PBMessage;
*/
import "C"

//export FreePBMessage
func FreePBMessage(message *C.PBMessage) {
    C.free(unsafe.Pointer(message.data))
    C.free(unsafe.Pointer(message))
}

//export Test
func Test(params *C.PBMessage) *C.PBMessage {
    paramsData := (*C.char)(params.data)
    paramsLength := params.len
    slice := (*[1 << 30]byte)(unsafe.Pointer(paramsData))[:paramsLength:paramsLength]

    person := &Person{}
    if err := proto.Unmarshal(slice, person); err != nil {
        panic(err)
    }

    out, err := proto.Marshal(person)
    if err != nil {
        panic(err)
    }

    pbMessageData := C.malloc(C.size_t(len(out)))

    pbMessageArray := (*[1 << 30]byte)(pbMessageData)
    copy(pbMessageArray[:], out)

    pbMessage := (*C.PBMessage)(C.malloc(C.size_t(unsafe.Sizeof(C.PBMessage{}))))
    pbMessage.len = (C.int)(len(out))
    pbMessage.data = pbMessageData

    return pbMessage
}

The JavaScript code:

const ffi = require('ffi-napi');
const ref = require('ref-napi')
const ArrayType = require('ref-array-di')(ref)
const StructType = require('ref-struct-di')(ref)

const PBMessageStruct = StructType({
    data: 'char*',
    len: 'int'
});

const lib = ffi.Library(path.join(__dirname, '../bindings.so'), {
    FreePBMessage: ['void', [ref.refType(PBMessageStruct)]],
    Test: [ref.refType(PBMessageStruct), ['char*']]
});

let i = 0;
while (i++ < 400) {
    const me = new ogamePb.Person({ name: 'My Name', age: 26 });
    const mePb = ogamePb.Person.encode(me).finish();

    const pbMessage = new PBMessageStruct();
    pbMessage.data = mePb;
    pbMessage.len = mePb.length;

    //==> THIS LINE SEEMS TO BE THE FIRST TO ACTUALLY CAUSE A LEAK
    //    Valgrind: definitely lost: 260,688 bytes
    let resRef = current.Test(pbMessage.ref())

    //==> THIS GROUP OF THREE LINES LEAKS TOO, BUT LESS
    //    If I uncomment it: Valgrind: definitely lost: 292,688 bytes
    // const res = resRef.deref();
    // const data = ref.reinterpret(res.data, res.len);
    //
    // const person = ogamePb.Person.decode(data);

    //==> THIS IS MEANT TO FREE MEMORY
    //    But ironically, it's better when I comment it out: Valgrind: definitely lost: 171,088 bytes
    current.FreePBMessage(resRef);

    console.log(i, process.memoryUsage().rss / 1024 / 1024);
}

Please note the comments in caps lock. The most ironic thing is that my attempt to manually free some memory makes it worse.

I've confirmed with an infinite loop that the program ends up crashing with a V8 OOM crash after having engulfed gigabytes of memory.

I have tested the code with Valgrind:

valgrind --leak-check=full -v node dist/apps/server/main.js

You can check out the full Valgrind log here: https://pastebin.com/ZjCiNqm0

Unfortunately I have very limited experience with unmanaged memory or ffi so I'm not able to conclude anything useful from the analysis, but just confirm that I do have some kind of leak.

My code is inspired from https://github.com/redredgroovy/go-ffi-demo, but I don't know if its author checked for leaks. Notable difference: it uses ffi while I use this library. I have also tested the exact same code but with ffi-cross and the same issue appears.

This issue is more a question, a call for help, but I thought I could still post here in case this actually hides a bug in the library.

toverux commented 3 years ago

Sorry, wrong repo