mirror of
https://github.com/ddnet/ddnet.git
synced 2024-11-09 09:38:19 +00:00
Fix race conditions in job state handling
We want to change the state of the job from `STATE_QUEUED` to `STATE_RUNNING`, but not from `STATE_ABORTED`. Previously, the code usedd `exchange` to change the state and compared non-atomically afterwards. This led to `STATE_ABORTED` being replaced by `STATE_RUNNING` in a race. With `compare_exchange_strong`, this can no longer happen. Fixes #8044.
This commit is contained in:
parent
44bf0134f0
commit
e7717f2265
|
@ -83,9 +83,8 @@ void CJobPool::RunLoop()
|
||||||
|
|
||||||
if(pJob)
|
if(pJob)
|
||||||
{
|
{
|
||||||
// do the job if we have one
|
IJob::EJobState OldStateQueued = IJob::STATE_QUEUED;
|
||||||
const IJob::EJobState OldStateQueued = pJob->m_State.exchange(IJob::STATE_RUNNING);
|
if(!pJob->m_State.compare_exchange_strong(OldStateQueued, IJob::STATE_RUNNING))
|
||||||
if(OldStateQueued != IJob::STATE_QUEUED)
|
|
||||||
{
|
{
|
||||||
if(OldStateQueued == IJob::STATE_ABORTED)
|
if(OldStateQueued == IJob::STATE_ABORTED)
|
||||||
{
|
{
|
||||||
|
@ -109,10 +108,13 @@ void CJobPool::RunLoop()
|
||||||
}
|
}
|
||||||
|
|
||||||
// do not change state to done if job was not completed successfully
|
// do not change state to done if job was not completed successfully
|
||||||
const IJob::EJobState OldStateRunning = pJob->m_State.exchange(IJob::STATE_DONE);
|
IJob::EJobState OldStateRunning = IJob::STATE_RUNNING;
|
||||||
if(OldStateRunning != IJob::STATE_RUNNING)
|
if(!pJob->m_State.compare_exchange_strong(OldStateRunning, IJob::STATE_DONE))
|
||||||
{
|
{
|
||||||
pJob->m_State = OldStateRunning;
|
if(OldStateRunning != IJob::STATE_ABORTED)
|
||||||
|
{
|
||||||
|
dbg_assert(false, "Job state invalid, must be either running or aborted");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
else if(m_Shutdown)
|
else if(m_Shutdown)
|
||||||
|
|
Loading…
Reference in a new issue