m1dugh / native-sound-mixer

MIT License
25 stars 13 forks source link

Native Crash #20

Closed hrueger closed 1 year ago

hrueger commented 1 year ago

Hi, when trying to use this lib on a Windows 10 machine, my application crashed with the following error:

FATAL ERROR: HandleScope::HandleScope Entering the V8 API without proper locking in place
 1: 00007FF6A265B34F v8::internal::CodeObjectRegistry::~CodeObjectRegistry+123599
 2: 00007FF6A25E8CB6 v8::internal::MicrotaskQueue::GetMicrotasksScopeDepth+65206
 3: 00007FF6A25E9D8D node::OnFatalError+301
 4: 00007FF6A2F10A0B v8::HandleScope::Initialize+107
 5: 00007FF6A2EFAFF1 v8::EscapableHandleScope::EscapableHandleScope+81
 6: 00007FF6A262CC78 napi_open_escapable_handle_scope+120
 7: 00007FF9CA33280D Napi::FunctionReference::New+61 [A:\_Source\native-sound-mixer\node_modules\node-addon-api\napi-inl.h]:L3370
 8: 00007FF9CA333663 SoundMixer::DeviceObject::New+83 [A:\_Source\native-sound-mixer\cppsrc\win\sound-mixer.cpp]:L83
 9: 00007FF9CA333465 SoundMixer::MixerObject::GetDevices+341 [A:\_Source\native-sound-mixer\cppsrc\win\sound-mixer.cpp]:L4710: 00007FF9CA336A3E Napi::details::WrapCallback<<lambda_2d6a469e53703925d71c48cc7e420a87> >+46 [A:\_Source\native-sound-mixer\node_modules\node-addon-api\napi-inl.h]:L75
11: 00007FF9CA335F58 Napi::details::TemplatedCallback<&SoundMixer::MixerObject::GetDevices>+40 [A:\_Source\native-sound-mixer\node_modules\node-addon-api\napi-inl.h]:L153
12: 00007FF6A262564B node::Stop+32747
13: 00007FF6A2F3E74F v8::internal::SetupIsolateDelegate::SetupHeap+53823
14: 00007FF6A2F6FAE9 v8::internal::SetupIsolateDelegate::SetupHeap+255449
15: 000002D94CCBDBBE

Any idea?

m1dugh commented 1 year ago

could you show me your node code causing this crash ?

hrueger commented 1 year ago

Thanks for the fast response. I'll try to find the exact call. It's a pretty complex project, it might take a while.

hrueger commented 1 year ago

So, I've finally found it. This is the code:

const SoundMixer = require("native-sound-mixer").default;
const pollInterval = setInterval(() => {
    for (const device of SoundMixer.devices) {
        console.log(device, device.volume);
    }
}, 500);

It works find for the first 6 iterations, then in the 7th one it crashes. However, I was not able to reproduce that in a standalone example, I don't know why. It is definitely that line, though, as it works fine if I comment it out. It does not matter if I read the mute or the volume property by the way.

m1dugh commented 1 year ago

Are you using the call in a multi threaded context ? It looks like the problem is the same as this one https://github.com/nodejs/node-addon-api/issues/779. I can hardly fix the code right now, i'll try to make it as fast as I can.

hrueger commented 1 year ago

I am using Worker Threads on my application, but this library is used in the main thread.

m1dugh commented 1 year ago

I might have located the problem, i'll try to fix it as soon as possible.

hrueger commented 1 year ago

Thanks in advance!

m1dugh commented 1 year ago

Since the bug is not reproductible in a standalone, it will be hard to test the code on my side. I can still push the fix on develop and you can test it before I push a new version if that's ok for you ๐Ÿ˜„. Otherwise, I will push the fix to a new version on npm registry but I can't be sure it will fix the problem ...

hrueger commented 1 year ago

I can still push the fix on develop and you can test it before I push a new version if that's ok for you

Sure ๐Ÿ‘

m1dugh commented 1 year ago

It took longer than I expected, I should be done by next week I guess.

hrueger commented 1 year ago

No problem, thanks for your efforts ๐Ÿ‘

m1dugh commented 1 year ago

I pushed something to 20-native-crash branch, I have no clue whether it might be better but you can try anyway.

hrueger commented 1 year ago

Hm, I can't even reproduce the original error anymore... Very strange.

However, while testing, I found that there may be a memory leak? Not sure, though. If you run the following code, you can see the memory usage of the Node Process increasing by about 1MB / s. Maybe this gives a hint? Could also be completely unreleated, though ;-)

const nsm = require("native-sound-mixer").default;

let mute;
let volume;
(async () => {
    while (true) {
        console.log("next round")
        for (let i = 0; i < 1000; i++) {
            for (const device of nsm.devices) {
                mute = device.mute;
                volume = device.volume;
            }
        }
        await new Promise((resolve) => { setTimeout(() => { resolve(); }, 500); });
    }
})();
m1dugh commented 1 year ago

If you can't reproduce the original error, I hope it was some strange node issue. Concerning the memory leak problem, I'm currently working on it. However, due to the way node handles memory, the memory should be flushed regularly, fixing the memory leak as it goes.

hrueger commented 1 year ago

I hope it was some strange node issue

I agree ๐Ÿ‘

Concerning the memory leak problem, I'm currently working on it.

Thanks a lot

m1dugh commented 1 year ago

The memory leaks concerning the devices should be fixed on develop (for windows only)

hrueger commented 1 year ago

Yes, can confirm that the memory leak is gone ๐Ÿ‘ Thanks for your effort.