Skip to content

Commit

Permalink
Moved Unity timer checks to the main thread to avoid race conditions.
Browse files Browse the repository at this point in the history
  • Loading branch information
@timcassell
timcassell committed Dec 14, 2024
1 parent 4d4a312 commit 3e3c95e
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 48 deletions.
71 changes: 47 additions & 24 deletions Package/UnityHelpers/2018.3/UnityRealTimerFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ private sealed class UnityRealTimerFactoryTimer : Internal.PromiseRefBase.Promis
private float _period;
// Start with 1 instead of 0 to reduce risk of false positives.
private int _version = 1;
private int _retainCounter;
private byte _retainCounter;
private bool _isTicking;
volatile private bool _isDisposed;
private bool _isDisposed;

internal int Version => _version;

Expand Down Expand Up @@ -111,14 +111,27 @@ internal override void MaybeDispose()

[MethodImpl(Internal.InlineOption)]
private void Retain()
=> Internal.InterlockedAddWithUnsignedOverflowCheck(ref _retainCounter, 1);
{
#if PROMISE_DEBUG || PROTO_PROMISE_DEVELOPER_MODE
checked
#endif
{
++_retainCounter;
}
}

[MethodImpl(Internal.InlineOption)]
private void Release()
{
if (Internal.InterlockedAddWithUnsignedOverflowCheck(ref _retainCounter, -1) == 0)
#if PROMISE_DEBUG || PROTO_PROMISE_DEVELOPER_MODE
checked
#endif
{
HandleNextInternal(Promise.State.Resolved);
if (--_retainCounter == 0)
{
HandleNextInternal(Promise.State.Resolved);

}
}
}

Expand Down Expand Up @@ -183,55 +196,65 @@ private void InvokeDirect()

public void Change(TimeSpan dueTime, TimeSpan period, int token)
{
Retain();
if (Volatile.Read(ref _version) != token)
{
Release();
throw new ObjectDisposedException(nameof(UnityRealTimerFactoryTimer));
}

if (InternalHelper.IsOnMainThread())
{
ChangeImpl(dueTime, period);
}
else
{
InternalHelper.PromiseBehaviour.Instance._syncContext.Send(
t => t.UnsafeAs<UnityRealTimerFactoryTimer>().ChangeImpl(dueTime, period),
this
);
ChangeImpl(dueTime, period, token);
return;
}

Promise.Run((this, dueTime, period, token),
cv => cv.Item1.ChangeImpl(cv.dueTime, cv.period, cv.token),
InternalHelper.PromiseBehaviour.Instance._syncContext, forceAsync: true
).Wait();
}

private void ChangeImpl(TimeSpan dueTime, TimeSpan period)
private void ChangeImpl(TimeSpan dueTime, TimeSpan period, int token)
{
if (_isDisposed | _version != token)
{
throw new ObjectDisposedException(nameof(UnityRealTimerFactoryTimer));
}

if (dueTime == Timeout.InfiniteTimeSpan)
{
_dueTime = _period = float.PositiveInfinity;
Release();
return;
}

_dueTime = (float) (UnityEngine.Time.realtimeSinceStartup + dueTime.TotalSeconds);
_period = period == Timeout.InfiniteTimeSpan ? float.PositiveInfinity : (float) period.TotalSeconds;
if (_isTicking)
{
Release();
return;
}

Retain();
_isTicking = true;
// TODO: Optimize this to just add the await instruction to the processor directly without using a Promise.
new AwaitInstruction(this).ToPromise().Forget();
}

public Promise DisposeAsync(int token)
{
if (Interlocked.CompareExchange(ref _version, unchecked(token + 1), token) != token)
if (InternalHelper.IsOnMainThread())
{
return DisposeAsyncImpl(token);
}

return Promise.Run((this, token),
cv => cv.Item1.DisposeAsyncImpl(cv.token),
InternalHelper.PromiseBehaviour.Instance._syncContext, forceAsync: true
);
}

private Promise DisposeAsyncImpl(int token)
{
if (_isDisposed | _version != token)
{
throw new ObjectDisposedException(nameof(UnityRealTimerFactoryTimer));
}
_isDisposed = true;
unchecked { _version = token + 1; }

Release();
return new Promise(this, Id);
Expand Down
71 changes: 47 additions & 24 deletions Package/UnityHelpers/2018.3/UnitySimulatedTimerFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ private sealed class UnitySimulatedTimerFactoryTimer : Internal.PromiseRefBase.P
private float _period;
// Start with 1 instead of 0 to reduce risk of false positives.
private int _version = 1;
private int _retainCounter;
private byte _retainCounter;
private bool _isTicking;
volatile private bool _isDisposed;
private bool _isDisposed;

internal int Version => _version;

Expand Down Expand Up @@ -111,14 +111,27 @@ internal override void MaybeDispose()

[MethodImpl(Internal.InlineOption)]
private void Retain()
=> Internal.InterlockedAddWithUnsignedOverflowCheck(ref _retainCounter, 1);
{
#if PROMISE_DEBUG || PROTO_PROMISE_DEVELOPER_MODE
checked
#endif
{
++_retainCounter;
}
}

[MethodImpl(Internal.InlineOption)]
private void Release()
{
if (Internal.InterlockedAddWithUnsignedOverflowCheck(ref _retainCounter, -1) == 0)
#if PROMISE_DEBUG || PROTO_PROMISE_DEVELOPER_MODE
checked
#endif
{
HandleNextInternal(Promise.State.Resolved);
if (--_retainCounter == 0)
{
HandleNextInternal(Promise.State.Resolved);

}
}
}

Expand Down Expand Up @@ -183,32 +196,28 @@ private void InvokeDirect()

public void Change(TimeSpan dueTime, TimeSpan period, int token)
{
Retain();
if (Volatile.Read(ref _version) != token)
{
Release();
throw new ObjectDisposedException(nameof(UnitySimulatedTimerFactoryTimer));
}

if (InternalHelper.IsOnMainThread())
{
ChangeImpl(dueTime, period);
}
else
{
InternalHelper.PromiseBehaviour.Instance._syncContext.Send(
t => t.UnsafeAs<UnitySimulatedTimerFactoryTimer>().ChangeImpl(dueTime, period),
this
);
ChangeImpl(dueTime, period, token);
return;
}

Promise.Run((this, dueTime, period, token),
cv => cv.Item1.ChangeImpl(cv.dueTime, cv.period, cv.token),
InternalHelper.PromiseBehaviour.Instance._syncContext, forceAsync: true
).Wait();
}

private void ChangeImpl(TimeSpan dueTime, TimeSpan period)
private void ChangeImpl(TimeSpan dueTime, TimeSpan period, int token)
{
if (_isDisposed | _version != token)
{
throw new ObjectDisposedException(nameof(UnitySimulatedTimerFactoryTimer));
}

if (dueTime == Timeout.InfiniteTimeSpan)
{
_dueTime = _period = float.PositiveInfinity;
Release();
return;
}

Expand All @@ -218,22 +227,36 @@ private void ChangeImpl(TimeSpan dueTime, TimeSpan period)
_period = period == Timeout.InfiniteTimeSpan ? float.PositiveInfinity : (float) period.TotalSeconds;
if (_isTicking)
{
Release();
return;
}

Retain();
_isTicking = true;
// TODO: Optimize this to just add the await instruction to the processor directly without using a Promise.
new AwaitInstruction(this).ToPromise().Forget();
}

public Promise DisposeAsync(int token)
{
if (Interlocked.CompareExchange(ref _version, unchecked(token + 1), token) != token)
if (InternalHelper.IsOnMainThread())
{
return DisposeAsyncImpl(token);
}

return Promise.Run((this, token),
cv => cv.Item1.DisposeAsyncImpl(cv.token),
InternalHelper.PromiseBehaviour.Instance._syncContext, forceAsync: true
);
}

private Promise DisposeAsyncImpl(int token)
{
if (_isDisposed | _version != token)
{
throw new ObjectDisposedException(nameof(UnitySimulatedTimerFactoryTimer));
}
_isDisposed = true;
unchecked { _version = token + 1; }

Release();
return new Promise(this, Id);
Expand Down

0 comments on commit 3e3c95e

Please sign in to comment.