kometbomb / prototracker-modular

A modular synth tracker
MIT License
46 stars 5 forks source link

Bugfind #30

Closed cupcake99 closed 5 years ago

cupcake99 commented 5 years ago

Hey Tero, been a while. Hope you're well and the cats are happy.

I found a bug last year on some work you did with the threading in the Mixer class. The program was crashing on quit when i built off your new code, and i couldn't work out why. Gave up for a while but thought i might as well get around to having a look into it. Seems the thread needs to be set as NULL to avoid being called again after getting released. Very simple fix, but i can't see why it would be called more than once in the first place. I hope i've got it right and there's no resource leaks or anything as a result. It should be safe if the thread was being released anyway and the pointer was just hanging around and causing the problem.

It was puzzling at first as i never had the same fault with the code before your changes in 39a0ea8dc, even though the deinitAudio() method was exactly the same... then i realised that it was releasing a null thread to begin with as there was no initialising call to SDL_CreateThread()! Hence no crash. SDL docs say about SDL_WaitThread() "It is safe to pass a NULL thread to this function; it is a no-op. "

Ok, bye bye. I'm keen to work on this project again now - feeling all pleased with myself for figuring this out.

kometbomb commented 5 years ago

Good find. The deconstructor calls deinitAudio() as does stopThread() (which is called by something else outside the Mixer class). I'll make this change in the main repo and merge it here.

kometbomb commented 5 years ago

OK, merged in the latest PR. Thanks!

cupcake99 commented 5 years ago

Thanks, the refactoring looks great!

It's going to prove useful being able to turn off audio threading in the pre-procesor as i've been finding it increases CPU usage by about 3x on my macbook, so reverted the changes on my experimental branch. Will be able to converge them more cleanly now. Nice.