crash on windows 10, node 10

[checksummaster]

At the event "close", my application exit. to fix that, i remove "delete ao->device;" (see below) but I'm not sure why .., maybe will leak but working.

NAN_METHOD(Close) { Nan::EscapableHandleScope scope; audio_output_t ao = UnwrapPointer<audio_output_t >(info[0]); ao->close(ao); int r = 0; if (ao->deinit) { r = ao->deinit(ao); } //delete ao->device; info.GetReturnValue().Set(scope.Escape(Nan::New(r))); }

[JonSilver]

@LinusU, @TooTallNate I'm still experiencing this behaviour even with the new v0.5.0. The app plays one stream, piped from lame, then exits with no error. Breakpointing on all or uncaught exceptions doesn't break. If the pipe from lame goes nowhere, bypassing speaker, the app remains running.

I've tried versions back to 0.3.1, where it worked, and have ascertained that this misbehaviour appeared between 0.4.0, with which it all worked fine, and 0.4.1 with which the app prematurely exits.

• Fresh test app
  • play an MP3 from a file stream via lame 3 times at 2 second intervals in a setInterval
• fresh node_modules & package-lock.json
• Node 10.17.0
• Windows 10

[LinusU]

I'm sorry to hear that, unfortunately I don't have access to a Windows 10 computer to try on 🤔

Would you be able to try commenting this line out:

https://github.com/TooTallNate/node-speaker/blob/8dc9e9cbc042e88a108c3381adac958f23cb4369/src/binding.c#L157

If that makes it work, we are probably double freeing that one. Just removing the free is probably the right thing to do, but it might be good to look at the source code for mpg123 to make sure that it's actually freeing it for us...

[JonSilver]

OK so I commented out the line, ran an npm rebuild speaker, and the test app worked. Is there a test that can ascertain whether it should be freed?

[LinusU]

Hmm, potentially this is not a double free, but rather that we are trying to free a static string 😁

https://github.com/TooTallNate/node-speaker/blob/20627e12f2628da8313544f5a92813f78cb3e68e/deps/mpg123/src/output/win32.c#L61

I need to dig deeper into this, potentially every backend works differently 😭

Hopefully I'll get some time this weekend! Thank you for testing 👏

[LinusU]

Hmm, since the open call in the win32 output always overwrites the current device, I guess that it's leaking that memory already 🤷‍♂

Maybe the best thing for us to do is store the pointer to the device string out of band, and then free it after we've closed the ao instance 🤔

[JonSilver]

Sounds like a good plan. :)

[LinusU]

@JonSilver could you try out #139 and see if that solves the problem for you? ☺️

[JonSilver]

@LinusU Patch applied, rebuilt & tested... That seems to fix things nicely :)

[LinusU]

Awesome, I'll push a new release!

[LinusU]

Released in 🚢 0.5.1 / 2019-12-16

[JonSilver]

Cool... I'll await the npmjs.com release :)

[LinusU]

It's already live 😉