From 927bdccf6ed9c320b8bc4c34bcfb4d36fef2912a Mon Sep 17 00:00:00 2001 From: mischa Date: Mon, 23 Sep 2024 11:10:56 +0200 Subject: [PATCH] feature(WorkerThread): Tick() returns bool to allow the thread function to stop the thread gracefully --- Assets/Mirror/Core/Threading/WorkerThread.cs | 10 +++++--- .../Editor/Threading/WorkerThreadTests.cs | 24 +++++++++++++++---- .../Transports/Threaded/ThreadedTransport.cs | 5 +++- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/Assets/Mirror/Core/Threading/WorkerThread.cs b/Assets/Mirror/Core/Threading/WorkerThread.cs index 4b037c0bf..f570dcc4d 100644 --- a/Assets/Mirror/Core/Threading/WorkerThread.cs +++ b/Assets/Mirror/Core/Threading/WorkerThread.cs @@ -21,8 +21,10 @@ public class WorkerThread // callbacks need to be set after constructor. // inheriting classes can't pass their member funcs to base ctor. // don't set them while the thread is running! + // -> Tick() returns a bool so it can easily stop the thread + // without needing to throw InterruptExceptions or similar. public Action Init; - public Action Tick; + public Func Tick; public Action Cleanup; public WorkerThread(string identifier) @@ -104,7 +106,7 @@ public bool StopBlocking(float timeout) // always define them, and make them call actions. // those can be set at any time. void OnInit() => Init?.Invoke(); - void OnTick() => Tick?.Invoke(); + bool OnTick() => Tick?.Invoke() ?? false; void OnCleanup() => Cleanup?.Invoke(); // guarded wrapper for thread code. @@ -128,7 +130,9 @@ public void Guard(string identifier) // run thread func while active while (active) { - OnTick(); + // Tick() returns a bool so it can easily stop the thread + // without needing to throw InterruptExceptions or similar. + if (!OnTick()) break; } } // Thread.Interrupt() will gracefully raise a InterruptedException. diff --git a/Assets/Mirror/Tests/Editor/Threading/WorkerThreadTests.cs b/Assets/Mirror/Tests/Editor/Threading/WorkerThreadTests.cs index 96bc91809..001e09c38 100644 --- a/Assets/Mirror/Tests/Editor/Threading/WorkerThreadTests.cs +++ b/Assets/Mirror/Tests/Editor/Threading/WorkerThreadTests.cs @@ -35,7 +35,7 @@ public void Callbacks() int tickCalled = 0; int cleanupCalled = 0; void Init() => Interlocked.Increment(ref initCalled); - void Tick() => Interlocked.Increment(ref tickCalled); + bool Tick() { Interlocked.Increment(ref tickCalled); return true; } void Cleanup() => Interlocked.Increment(ref cleanupCalled); // ctor runs thread and calls callback immediately @@ -82,7 +82,7 @@ public void ExceptionInTick() // exceptions in threads are silent by default. // guarantee we try/catch them. int cleanupCalled = 0; - void Tick() => throw new Exception("Test Exception"); + bool Tick() => throw new Exception("Test Exception"); void Cleanup() => Interlocked.Increment(ref cleanupCalled); // ctor runs thread and calls callback immediately @@ -103,6 +103,7 @@ public void IsAlive() { thread = new WorkerThread("WorkerThreadTests"); Assert.False(thread.IsAlive); + thread.Tick = () => true; thread.Start(); Thread.Sleep(10); @@ -111,13 +112,26 @@ public void IsAlive() Assert.False(thread.IsAlive); } + [Test] + public void TickIndicatesEnd() + { + thread = new WorkerThread("WorkerThreadTests"); + Assert.False(thread.IsAlive); + // returning false should automatically stop the thread + thread.Tick = () => false; + thread.Start(); + + Thread.Sleep(10); + Assert.False(thread.IsAlive); + } + // make sure stop returns immediately, but does stop it eventually [Test] public void SignalStop() { thread = new WorkerThread("WorkerThreadTests"); Assert.False(thread.IsAlive); - thread.Tick = () => Thread.Sleep(50); + thread.Tick = () => { Thread.Sleep(50); return true; }; thread.Start(); // stop should return immediately, while thread is shutting down @@ -135,7 +149,7 @@ public void StopBlocking() { thread = new WorkerThread("WorkerThreadTests"); Assert.False(thread.IsAlive); - thread.Tick = () => Thread.Sleep(50); + thread.Tick = () => { Thread.Sleep(50); return true; }; thread.Start(); // stop should wait until fully stopped @@ -149,7 +163,7 @@ public void StopBlocking_Deadlocked() { thread = new WorkerThread("WorkerThreadTests"); Assert.That(thread.IsAlive, Is.False); - thread.Tick = () => Thread.Sleep(5000); + thread.Tick = () => { Thread.Sleep(5000); return true; }; thread.Start(); // wait for it to start diff --git a/Assets/Mirror/Transports/Threaded/ThreadedTransport.cs b/Assets/Mirror/Transports/Threaded/ThreadedTransport.cs index e760b8464..cbf11bd51 100644 --- a/Assets/Mirror/Transports/Threaded/ThreadedTransport.cs +++ b/Assets/Mirror/Transports/Threaded/ThreadedTransport.cs @@ -263,7 +263,9 @@ void ProcessThreadQueue() } } - void ThreadTick() + // Tick() returns a bool so it can easily stop the thread + // without needing to throw InterruptExceptions or similar. + bool ThreadTick() { // early update the implementation first ThreadedClientEarlyUpdate(); @@ -279,6 +281,7 @@ void ThreadTick() // save some cpu power. // TODO update interval and sleep extra time would be ideal Thread.Sleep(1); + return true; } // threaded callbacks to call from transport thread.