FamiTracker doesn't consistently acquire mutexes when the GUI and audio threads operate on CSoundGen. This results in data races, and in the case of the VRC7, the GUI or audio thread can call CAPU::ChangeMachineRate > CVRC7::SetSampleSpeed > OPLL_delete, before the other thread calls OPLL_delete or OPLL_reset.

The use-after-free results in a crash on ASAN builds (link), and can be reliably reproduced by adding a 1-second sleep in CVRC7::SetSampleSpeed(), between calling OPLL_delete() and OPLL_new(). See https://github.com/nyanpasu64/Dn-FamiTracker/tree/asan-fix-vrc7 for a reproducer.

Fix

I added extra locks to CSoundGen::m_csAPULock when necessary. I'm not sure that the current set of locks is sufficient, but ASAN is no longer reporting errors when I open or switch documents, change module properties, or press F12.

Additionally I switched CSoundGen::m_csAPULock from CCriticalSection and CSingleLock (which didn't work properly), to std::mutex and std::unique_lock. I'm not sure why CSingleLock didn't work. Maybe it's because Lock() moves upon return (in the absence of NRVO), and the moved-from object's destructor unlocks the CCriticalSection. Maybe the lock is released upon std::this_thread::sleep_for(). It's not because I tried to lock the same mutex twice in the same thread (since MSVC's std::mutex throws a std::system_error in that case). In any case, std::unique_lock works properly.

Discussion

Theoretically it's safer to null out m_pOPLLInt before calling OPLL_new (it would be more exception-safe, except not because OPLL_new doesn't throw exceptions), but doing so would prevent ASAN from catching the data races (Windows MSVC doesn't have TSAN right now).

I'm not actually sure if the std::atomic is necessary. Not changing it is technically a data race, but not a big deal. It might be faster to use relaxed access, and switch the compound assignments to something that doesn't write then read(? unsure).

There is no wasted write-then-read. See https://en.cppreference.com/w/cpp/atomic/atomic/operator%3D:

Unlike most assignment operators, the assignment operators for atomic types do not return a reference to their left-hand arguments. They return a copy of the stored value instead.