How to write thread-safe C# code for Unity3D?

Although you might use volatile keyword for your bool flag, it does not always guarantee a thread-safe access to the field.

In your case I'd probably create a separate class Worker and use events to notify when background task completes execution:

// Change this class to contain whatever data you need public class MyEventArgs { public string Data { get; set; } } public class Worker { public event EventHandler<MyEventArgs> WorkComplete = delegate { }; private readonly object _locker = new object(); public void Start() { new Thread(DoWork).Start(); } void DoWork() { // add a 'lock' here if this shouldn't be run in parallel Thread.Sleep(5000); // ... massive computation WorkComplete(this, null); // pass the result of computations with MyEventArgs } } class MyClass { private readonly Worker _worker = new Worker(); public MyClass() { _worker.WorkComplete += OnWorkComplete; } private void OnWorkComplete(object sender, MyEventArgs eventArgs) { // Do something with result here } private void Update() { _worker.Start(); } } 

Feel free to change the code according to your needs

P.S. Volatile is good performance-wise, and in your scenario it should work as it looks like you get your reads and writes in the right order. Possibly the memory barrier is achieved precisely by reading/writing freshly - but there is no guarantee by MSDN specifications. It's up to you to decide whether to take the risk of using volatile or not.

Maybe you don't even need your _done variable, as you could achieve the same behaviour if you use the thread's IsAlive()-method. (given that you only have 1 background thread)

Like this:

if(_thread == null || !_thread.IsAlive()) { _thread = new Thread(Work); _thread.Start(); } 

I didn't test this btw .. this is just a suggestion :)

Use MemoryBarrier()

System.Threading.Thread.MemoryBarrier() is the right tool here. The code may look clunky, but it's faster than the other viable alternatives.

bool _isDone = false; public bool IsDone { get { System.Threading.Thread.MemoryBarrier(); var toReturn = this._isDone; System.Threading.Thread.MemoryBarrier(); return toReturn; } private set { System.Threading.Thread.MemoryBarrier(); this._isDone = value; System.Threading.Thread.MemoryBarrier(); } } 

Don't use volatile

volatile doesn't prevent an older value from being read, so it doesn't meet the design objective here. See Jon Skeet's explanation or Threading in C# for more.

Note that volatile may appear to work in many cases due to undefined behavior, specifically a strong memory model on many common systems. However, reliance on undefined behavior can cause bugs to appear when you're running your code on other systems. A practical example of this would be if you're running this code on a Raspberry Pi (now possible due to .NET Core!).

Edit: After discussing the claim that "volatile won't work here", it's unclear exactly what the C# spec guarantees; arguably, volatile might be guaranteed to work, albeit it with a larger delay. MemoryBarrier() is still the better solution since it ensures a faster commit. This behavior is explained in an example from "C# 4 in a Nutshell", discussed in "Why do I need a memory barrier?".

Don't use locks

Locks are a heavier mechanism meant for stronger process control. They're unnecessarily clunky in an application like this.

The performance hit is small enough that you probably won't notice it with light use, but it's still sub-optimal. Also, it can contribute (even if slightly) toward larger problems like thread starvation and deadlocking.

Details about why volatile doesn't work

To demonstrate the issue, here's the .NET source code from Microsoft (via ReferenceSource):

public static class Volatile { public static bool Read(ref bool location) { var value = location; Thread.MemoryBarrier(); return value; } public static void Write(ref byte location, byte value) { Thread.MemoryBarrier(); location = value; } } 

So, say that one thread sets _done = true;, then another reads _done to check if it's true. What does that look like if we inline it?

void WhatHappensIfWeUseVolatile() { // Thread #1: Volatile write Thread.MemoryBarrier(); this._done = true; // "location = value;" // Thread #2: Volatile read var _done = this._done; // "var value = location;" Thread.MemoryBarrier(); // Check if Thread #2 got the new value from Thread #1 if (_done == true) { // This MIGHT happen, or might not. // // There was no MemoryBarrier between Thread #1's set and // Thread #2's read, so we're not guaranteed that Thread #2 // got Thread #1's set. } } 

In short, the problem with volatile is that, while it does insert MemoryBarrier()'s, it doesn't insert them where we need them in this case.

Novices can create thread-safe code in Unity by doing the following:

• Copy the data they want a worker thread to work on.
• Tell a worker thread to work on the copy.
• From the worker thread, when the work is done, dispatch a call to the main thread to apply the changes.

This way you don't need locks and volatiles in your code, just two dispatchers (which hide all the locks and volatiles).

Now this is the simple and safe variant, the one novices should use. You're probably wondering what experts are doing: They do the exact same thing.

Here's some code from the Update method in one of my projects, which solves the very same problem you're trying to solve:

Helpers.UnityThreadPool.Instance.Enqueue(() => { // This work is done by a worker thread: SimpleTexture t = Assets.Geometry.CubeSphere.CreateTexture(block, (int)Scramble(ID)); Helpers.UnityMainThreadDispatcher.Instance.Enqueue(() => { // This work is done by the Unity main thread: obj.GetComponent<MeshRenderer>().material.mainTexture = t.ToUnityTexture(); }); }); 

Notice the only thing we have to do to make the above thread safe is to not edit blockor ID after calling enqueue. No volatile or explicit lock involved.

Here are the relevant methods from the UnityMainThreadDispatcher:

List<Action> mExecutionQueue; List<Action> mUpdateQueue; public void Update() { lock (mExecutionQueue) { mUpdateQueue.AddRange(mExecutionQueue); mExecutionQueue.Clear(); } foreach (var action in mUpdateQueue) // todo: time limit, only perform ~10ms of actions per frame { try { action(); } catch (System.Exception e) { UnityEngine.Debug.LogError("Exception in UnityMainThreadDispatcher: " + e.ToString()); } } mUpdateQueue.Clear(); } public void Enqueue(Action action) { lock (mExecutionQueue) mExecutionQueue.Add(action); } 

And here's a link to a thread pool implementation you can use until Unity finally supports the .NET ThreadPool: https://stackoverflow.com/a/436552/1612743

I do not think you need to add that much to your code. Unless Unity's version of Mono really does something hugely and codebreakingly different from the regular .Net Framework (which I am certain it does not), you'll not have different copies of _done in different threads in your scenario. So, no need for Interlocked, no need for volatile, no black magic.

What you really may encounter with a pretty high probability is a situation when your main thread sets _done to false in the very same moment when your background thread sets it to true, which is definitely a bad thing. That's why you need to surround those operations with 'lock's (don't forget to lock them on the SAME sync object).

As long as you use _done as you described in your code example, you can just 'lock' it every time you write your variable and everything will be fine.