Multithreading freezes editor

Questions & Answers
1

I outsourced my terrain generation into threads. The threads don’t use any unity api stuff and all variables which the thread and the main thread have access to are thread safed via a lock.

My problem is that after a few seconds (sometime 3-4s and sometimes 30s or 40s) the unity editor freezes.
If I open up the task manager the cpu and memory percentages are still changing (cpu is about 40-50% and memory about 300mb).

I’ve already tried unmanged threads and managed thread with the C# ThreadPool. The communication with the main thread goes over a status variable (which is locked) so if the task has finished the main thread collects the data and creates a mesh (every update the main thread checks for the status of the thread).

I’ve tried it on windows 10 with Unity 5.3/5.4 and on mac os x with unity 5.3/5.4.

Here’s the shortened code:

Edit: Updated code, now using volatile

public enum SurfaceCalculationStatus{
	NewJob,
	Processing,
	DataReady,
	Ready,
    Aborted
}

public class SurfaceCalculation {

	private SurfaceRenderType srt;
    
    //Some members...

	private volatile SurfaceCalculationStatus scs;


    public SurfaceCalculation(){

		scs = SurfaceCalculationStatus.Ready;

        ThreadPool.QueueUserWorkItem((o) => { ThreadFunction();});
    }

	public void Init(int x, int z, int size, int gridSize, SurfaceRenderType srt, Chunk c){
		//Some inits...

		scs = SurfaceCalculationStatus.NewJob;
		
	}

	private void ThreadFunction()
	{
		while (true) { 
            if(scs == SurfaceCalculationStatus.Aborted)
            {
                return;
            }
			if (scs == SurfaceCalculationStatus.NewJob) {
				scs = SurfaceCalculationStatus.Processing;
                
                //Heavy calculations    
					
            	scs = SurfaceCalculationStatus.DataReady;
            }

			Thread.Sleep (100);
		}
	}

	public void AbortThread(){
    	scs = SurfaceCalculationStatus.Aborted;
	}

    //Some Getter & Setters...

	public void SetReady(){
    	scs = SurfaceCalculationStatus.Ready;
    }

	public bool IsReady(){
        return scs == SurfaceCalculationStatus.Ready;
	}

	public bool IsData(){
        return scs == SurfaceCalculationStatus.DataReady;
    }
}

Edit: This is the class/logic which manages the threads. If I comment out the line

sc.SetReady ();

at the Update() method there is no freezing.

public class SurfaceFactory {

	private static List<Chunk>[] waitingQueue;
	private static SurfaceCalculation[] inProgressQueue;

	public static void Init(){
		waitingQueue = new List<Chunk>[WorldSettings.lvlRadii.Length];
		inProgressQueue = new SurfaceCalculation[SystemInfo.processorCount * 8];

		for (int i = 0; i < inProgressQueue.Length; i++) {
			inProgressQueue  *= new SurfaceCalculation ();*

  •  }*

  •  for (int i = 0; i < waitingQueue.Length; i++) {*

_ waitingQueue = new List ();_
* }*
* }*

* public static void CreateSurface(Chunk c){*
* if (GetFreeThreadCound() > 0) {*
* StartSurfaceCalculation (c);*
* } else {*
* waitingQueue[c.GetLOD()].Add (c);*
* }*
* }*

* public static void Update(){*
* for (int i = inProgressQueue.Length - 1; i >= 0; i–) {*
_ SurfaceCalculation sc = inProgressQueue ;
* if (sc.IsData ()) {
 //Thread finished*
/Chunk c = sc.GetChunk();
 Mesh mesh = new Mesh ();
 mesh.vertices = sc.GetVertices ();
 mesh.triangles = sc.GetIndices ();
 mesh.RecalculateNormals ();
c.SetMesh (mesh);/
* sc.SetReady ();
 }
 }*_

* int numberOfWaitingChunks = 0;*

* for (int i = 0; i < waitingQueue.Length; i++) {*
_ numberOfWaitingChunks += waitingQueue .Count;
* }*_

* int numberFreeThreads = GetFreeThreadCound ();*

* while (numberOfWaitingChunks > 0 && numberFreeThreads > 0){*
* for (int i = 0; i < waitingQueue.Length; i++) {*
_ if (waitingQueue .Count > 0) {
Chunk c = waitingQueue [0];
waitingQueue*.RemoveAt (0);
StartSurfaceCalculation (c);
numberFreeThreads–;
}
}
}
}*_

* public static void AbortThreads(){*
* for (int i = 0; i < inProgressQueue.Length; i++) {*
_ inProgressQueue .AbortThread ();
* }
}*_

* private static void StartSurfaceCalculation(Chunk c){*
* SurfaceCalculation sc = GetFreeThread ();*
* if (sc != null) {*
* sc.Init (c.GetChunkXMeters (false), c.GetChunkZMeters (false), c.GetSize (), c.GetGridSize (), c.GetSurfaceRenderType (), c);*
* } else {*
* waitingQueue [c.GetLOD ()].Add (c);*
* }*
* }*

* private static int GetFreeThreadCound(){*
* int c = 0;*
* for (int i = 0; i < inProgressQueue.Length; i++) {*
_ if (inProgressQueue .IsReady ())
* c++;
}
return c;
}*_

* private static SurfaceCalculation GetFreeThread(){*
* for (int i = 0; i < inProgressQueue.Length; i++) {*
_ if (inProgressQueue .IsReady ())
return inProgressQueue ;
* }
return null;
}
}*_

2

You never want to lock the whole code of the threaded function. That will lock every other thread that tries to aquire a lock while your function still runs. That completely destroys the purpose of using a thread in the first place and is prone for creating dead-locks.

How do you actually use that class? How do you managed those jobs? In such cases you usually don’t need any locks as long as you declare your state variable as volatile. As long as the sequence when someone is allowed to change the state of the variable is clear there are no locks required.

Note: This only works for variable read / write operations which are “atomic”. This is only given for primitive types (excluding double, long, ulong, decimal). Since the default underlying type for enums is “int”, reading and writing an enum is an atomic operation. “volatile” ensures that the compiler does not do any optimisations on that variable.

So as long as only one thread will write the variable everything is fine. In your case the order is clear:

state        Thread who has write-permission
-----------------------------------------
Ready        Main thread     --> can change to "NewJob" or "Aborted"
NewJob       Worker thread   --> can change to "Processing" or "DataReady"
Processing   Worker thread   --> can change to "DataReady"
DataReady    Main thread     --> can change to "Ready" or "NewJob" or "Aborted"

However to be on the safe side it’s better to declare seperate variables for commands and state. Commands are only set by the main thread and read by the worker and the state is only set by the worker and read by the main thread,

Anyways if you want to use a lock (maybe to allow setting the aborted state while still processing), make sure you hold the lock as short as possible.

 lock (locker)
 {
     scs = SurfaceCalculationStatus.Processing;
 }
 
 //Heavy calculations
 
 lock (locker)
 {
     if (scs ==SurfaceCalculationStatus.Aborted) // important to not overwrite an external abort request.
         return;
     scs = SurfaceCalculationStatus.DataReady;
 }

Also keep in mind that you always have to terminate the threads when running inside the editor. The playmode does not affect running threads. This can lead to massive problems inside the editor.

It’s generally not recommended to use Thread.Abort, however in the case of Unity it might be a good idea as backup procedure.

It’s generally adviced to have a dedicated “abort” variable which should be checked by the worker frequently during operation to terminate the thread within a short time.