Fix voices being forced to update without valid properties

When starting a voice, the source ID was set before its first update struct was
provided, creating a small window where a listener or effect slot update could
force a voice to update without it having any valid properties to update with.
Supplying the update struct first would create a different race, where the
mixer could see a voice without a source but with an update struct, causing the
update struct to be 'freed' without being applied.

The fix here is to provide the update struct before setting the source ID, and
change the mixer to ignore update structs for voices without a source ID. This
can pseudo-orphan the updates that get set on a voice just as it stops, leaving
the struct unusable until the voice is used again, or the voice gets deleted
which will clear it. But it allows the update struct to stay in place and get
applied once the voice gets a source ID.
This commit is contained in:
Chris Robinson
2020-03-04 21:15:32 -08:00
parent 48b9b541ec
commit 78251fd7e6
2 changed files with 10 additions and 16 deletions
+3 -2
View File
@@ -91,6 +91,7 @@ ALvoice *GetSourceVoice(ALsource *source, ALCcontext *context)
return nullptr;
}
void UpdateSourceProps(const ALsource *source, ALvoice *voice, ALCcontext *context)
{
/* Get an unused property container, or allocate a new one as needed. */
@@ -505,10 +506,10 @@ void InitVoice(ALvoice *voice, ALsource *source, ALbufferlistitem *BufferList, A
std::for_each(voice->mChans.begin(), voice->mChans.begin()+voice->mNumChannels, init_nfc);
}
voice->mSourceID.store(source->id, std::memory_order_release);
source->PropsClean.test_and_set(std::memory_order_acq_rel);
UpdateSourceProps(source, voice, context);
voice->mSourceID.store(source->id, std::memory_order_release);
}
+7 -14
View File
@@ -1576,17 +1576,7 @@ void CalcAttnSourceParams(ALvoice *voice, const ALvoicePropsBase *props, const A
void CalcSourceParams(ALvoice *voice, ALCcontext *context, bool force)
{
ALvoiceProps *props{voice->mUpdate.exchange(nullptr, std::memory_order_acq_rel)};
if(voice->mSourceID.load(std::memory_order_relaxed) == 0)
{
/* Don't update voices that no longer have a source. But make sure any
* update struct it has is returned to the free list.
*/
if UNLIKELY(props)
AtomicReplaceHead(context->mFreeVoiceProps, props);
return;
}
if(!props && !force)
return;
if(!props && !force) return;
if(props)
{
@@ -1723,9 +1713,12 @@ void ProcessParamUpdates(ALCcontext *ctx, const ALeffectslotArray &slots,
for(ALeffectslot *slot : slots)
force |= CalcEffectSlotParams(slot, sorted_slots, ctx);
auto calc_params = [ctx,force](ALvoice *voice) -> void
{ CalcSourceParams(voice, ctx, force); };
std::for_each(voices.begin(), voices.end(), calc_params);
for(ALvoice *voice : voices)
{
/* Only update voices that have a source. */
if(voice->mSourceID.load(std::memory_order_relaxed) != 0)
CalcSourceParams(voice, ctx, force);
}
}
IncrementRef(ctx->mUpdateCount);
}