Fix random multithreaded crash that happens when setting the audio stream on a AudioStreamRandomPitch stream.

[jitspoe]

Added locks in AudioStreamRandomPitch::set_audio_stream. This is to fix #54866

[ellenhp]

Discussed in the issue. I don't want to merge this until we're sure we've solved the issue, and I don't like the idea of using the global audio server lock if we can avoid it.

[jitspoe]

Discussed in the issue. I don't want to merge this until we're sure we've solved the issue, and I don't like the idea of using the global audio server lock if we can avoid it.

I've run this for several hours with no crash, so I'm pretty sure the crash is fixed. The lock I added only happens when setting the stream, so it's not like it's going to happen every frame. Should be pretty rare. This lock was also used when setting streams in other places.

I could create a new mutex, but I have a few concerns:

1. Having multiple different locks acting on the same code increase the chance of a deadlock. Also adds complexity to the maintainability.
2. I'd have to add a new lock the mix, which WOULD have a performance impact every frame.
3. Might need to add that lock to other places that use the stream as well -- not sure what places outside of the mix use it. Might result in an even more difficult to reproduce crash.

Since the crash does not happen in master, whatever fix we do should probably be only put into 3.x.

[ellenhp]

I'll need to think about it. Just to respond to your points here:

1. Deadlocks only occur when multiple locks are acquired in multiple places in different orders, and we wouldn't be performing any function calls inside the locked segments so that's not a risk here I think.
2. I don't necessarily agree that the performance impact of a single mutex lock/unlock cycle (without contention) is worth thinking about or even measurable.
3. My issue here is I think it would be nice to eventually remove the global audio server lock. I don't think anyone can currently explain why this particular change solves the bug. Do you have theories? I've looked at the code and I can't figure it out. If we created a mutex local to this class it would be pretty obvious. It protects data private to that class, therefore it only needs to be locked and unlocked in that class's own methods, since those are the only places that private data could be modified. (EDIT: friend classes can also access private data, but you get the idea I think)

[jitspoe]

I think the goal of getting rid of the audio lock is fine in 4.x. This should probably be 3.x-specific, since I can't reproduce the bug in master.

As for how it works, the lock is applied in the audio driver thread_func(). Ex: void AudioDriverWASAPI::thread_func(void *p_udata) calls ad->lock(); before doing all the mixing and whatnot, so setting the lock when changing the stream ensures that the mixing doesn't happen while the stream is changing.

Also, every other place that sets the stream (in 3.x, anyway) calls the driver lock, so I'm not sure why this case should be different.

[akien-mga]

This would need to be redone for the 3.x branch as it's no longer applicable on master (AudioStreamRandomPitch was refactored into AudioStreamRandomizer and the locking system is different).

Based on the above discussion and that in #54866, it seems like despite some concerns this might be a good solution for 3.x. Could you make a new PR for 3.x?

[jitspoe]

Opened a new PR for 3.x as requested here: #96261