node-ffi-napi / node-ffi-napi

A foreign function interface (FFI) for Node.js, N-API style
MIT License
970 stars 136 forks source link

Callback is still getting garbage collected. #3

Open stuartbrussell-intuit opened 6 years ago

stuartbrussell-intuit commented 6 years ago

Our macOS electron app uses nodobjc with ffi, and we have been getting "ffi: callback has been garbage collected" errors. To detect the error, I added test code to our app that will loop forever making async calls from javascript, one at a time, waiting for completion each time. Somewhere between 0 and 200 iterations, we get the error, usually at 50 to 100, but it does seem randomish.

Last week I created a local "nodobjc-napi" on my machine, that uses ffi-napi 2.4.0, which has the workaround of Callback is garbage collected.

My test code still sees the same error with the same frequency. I searched my node_modules and app/node_modules for the original ffi, but all I see is ffi-napi, so as far as I can tell the workaround is active.

We use nodobjc for our authentication flow, and if it doesn't work, then about 2% of our customers (randomishly distributed) won't be able to sign in at any given time.

Is there some way I can help to diagnose this, or to be a test bed if anyone wants to experiment? I can try adding delays, additional logging, other code, I can stay up late and spend some time on weekends. What I can't do is actually solve the problem, because I lack the skill set. Help!

addaleax commented 6 years ago

@stuartbrussell-intuit Hm, I’d really like to help you, but I’m not sure what to do without a reproduction.

Generally, you’ll still need to know how callbacks are called from the native code, and make sure that the callback stays alive for as long as it is needed.

I guess it could be possible to extend the debug mode by something that takes a stack trace when a callback is allocated, then print that stack trace along with the error once something attempts to call the gc’ed callback. Does that sound useful to you?

stuartbrussell-intuit commented 6 years ago

@addaleax Thank you for responding! Yes, a stack trace might help. Like I said, I can make it happen at will, specifically every 50-100 times, but very consistently.

I will double check the callback lifetime in my JavaScript. I could be wrong, but my understanding was that it doesn't matter what the caller does. So, when ffi throws this exception, is ffi reporting that my JavaScript did the garbage collection? Or is ffi saying that some other code did the garbage collection?

Just a reminder, my JS doesn't call ffi directly. I'm calling nodobjc from my JS, which then calls ffi. I'm hoping that there will be a "nodobjc-napi" that uses this ffi-napi.

Thanks again. Any and all help is appreciated. :)

addaleax commented 6 years ago

So, when ffi throws this exception, is ffi reporting that my JavaScript did the garbage collection?

It is saying that the JS callback function, the one that FFI tried to call, was collected by the JS GC. (I’m not sure that’s a good answer but then again I’m not sure I understand the question?)

stuartbrussell-intuit commented 6 years ago

Maybe it wasn't a well crafted question. Here's another angle: Does ffi retain the callback, or is it a weak reference? My assumption was that ffi will retain it. Could be a bad assumption.

addaleax commented 6 years ago

@stuartbrussell-intuit It is a weak reference because it cannot know how long C is going to need access to it; so the alternative would be storing it forever, which would always lead to a memory leak.

atishay commented 4 years ago

https://github.com/node-ffi-napi/node-ffi-napi/pull/56 might be a fix for this.