Search Unity

Occasion Thread failure

Discussion in 'Scripting' started by Poo2thegeek, Aug 30, 2014.

  1. Poo2thegeek

    Poo2thegeek

    Joined:
    Aug 30, 2014
    Posts:
    3
    I don't know if this is primarily an occurrence in unity or simply a bug in my c# code, but when using threads to load or generate objects (chunks of a world) occasionally they fail. They are removed from the list of "toDo's" but do not generate/load.

    I once had this problem because I wasn't giving the new created thread time to start, which I fixed by adding a "Thread.sleep(1)" just after the new Thread start. However i still occasionally get chunks that fail to load or generate. I have a fall back method for this (check's if the chunk is still in the "toDo" list, if it isn't then give it up to 2 seconds to load or generate, then try add it to the "toDo" list again).

    Here is my current Thread code (this is for generation, but it is almost identical to to the loading code)

    Code (CSharp):
    1. private void ChunkGeneratorUpdate(){
    2.    
    3.         while (true) {
    4.            
    5.             if (toGenerate.Count > 0) {
    6.  
    7.                 for(int i=0; i<threads.Length; i++){
    8.                     if(threads[i] == null || !threads[i].IsAlive){
    9.  
    10.                         threads[i] = new Thread(() => Generate(toGenerate[0]));
    11.                         threads[i].Start();
    12.                        
    13.                         Thread.Sleep (1); //Give it a chance to do so.
    14.                         toGenerate.RemoveAt(0);
    15.                        
    16.                         if(toGenerate.Count == 0)
    17.                             break;
    18.                        
    19.                     }
    20.                 }
    21.                
    22.             } //Else continue;
    23.             Thread.Sleep (1);
    24.         }
    25.     }
    Chunk's are added to "toGenerate" via the method "GenerateChunk":
    Code (CSharp):
    1. public void GenerateChunk(Chunk chunk){
    2.  
    3.         if (toGenerate.Contains (chunk) && chunk.Generating) {
    4.             toGenerate.Remove(chunk); //Remove it incase an error has occured and to prevent the chunk being generated twice
    5.         }
    6.  
    7.         chunk.Generating = true;
    8.         chunk.Generated = false;
    9.         chunk.Loaded = true;
    10.         chunk.Loading = false;
    11.         toGenerate.Add (chunk);
    12.  
    13.     }
    These are the two bit's I would assume are important. I think may have something to do with Thread timing. But increasing the Thread.sleep doesn't seem to change much other than make it take longer to load/generate chunks.
    As mentioned before this is not fatal as I have a fall back method in my chunk update which checks to see if everything has gone correctly:
    Code (CSharp):
    1.         if (Generating && !((ChunkGenerator)(transform.parent.parent.FindChild("ChunkGenerator").GetComponent("ChunkGenerator"))).ContainsChunk(this)) {//Long line, just get's the generatror and checks if this chunk is in the "toGenerate" list.
    2.             ChunkGenerationTime_ += Time.deltaTime;
    3.             //print ( ChunkManager.ChunkPositionHash(this) + " generating: " + ChunkGenerationTime_);
    4.            
    5.             if(ChunkGenerationTime_ >= ChunkGenerator.ChunkGenerationTimeout && GenerateIfNeeded){ //If it's spent too long, and it is allowed to be generated..
    6.                 ((ChunkGenerator)(transform.parent.parent.FindChild("ChunkGenerator").GetComponent("ChunkGenerator"))).GenerateChunk(this);
    7.                 ChunkGenerationTime_ = 0;
    8.             }
    9.            
    10.             return;
    11.         }else{
    12.             ChunkGenerationTime_ = 0;
    13.         }
    Thanks for any help!
     
  2. Serge_Billault

    Serge_Billault

    Joined:
    Aug 26, 2014
    Posts:
    190
    Unity API is not thread safe. I made you a sample project to illustrate this fact : cylinders are built from within one of the 4 agents ( threaded tasks) availble from the service they belong to, while capsules are assets that are loaded

    Did you try.... this : https://www.sugarsync.com/pf/D2307301_98582733_602566

     
  3. cmberryau

    cmberryau

    Joined:
    Mar 6, 2013
    Posts:
    12
    In my experience you shouldn't need to sleep the thread before starting it. I've never had issues starting C# threads within Unity. How are you sure that the threads are failing? I find it hard to believe they are failing to begin.

    Try just throwing in a Debug.Log on each Thread start, and seeing if it is the actual thread failing to begin or if it is another problem later down the line.
     
  4. Serge_Billault

    Serge_Billault

    Joined:
    Aug 26, 2014
    Posts:
    190
    Maybe he stumbled across an old msdn or unity article on threads where it was advised to wait for a thread to be started (but not with a sleep thought) in some situations, But nowadays most systems internally wait for the thread to be in a running state. He might have to dig into his program workflow to detect any concurrent events that would need more synchronisation.
     
  5. Poo2thegeek

    Poo2thegeek

    Joined:
    Aug 30, 2014
    Posts:
    3
    You say that the unity API isn't safe, but what do I do to fix this? I'm new to C# and unity. Any recommendations on how to fix this if it is because of the threads not being safe?

    Also I do believe it is a problem with the threads starting, though I shall check this when I get home.
    I saw in a code snippet somewhere at one point someone making the thread sleep to "//give the new thread a chance to start".

    Any ideas on how to fix this if it is due to Unity's lack of thread safe API?
     
  6. Serge_Billault

    Serge_Billault

    Joined:
    Aug 26, 2014
    Posts:
    190
    Unity API not being thread safe means that any attempt to call the API from a thread will generally raise an exception wich will prevent the program to execute normally afterward (just check the console errors log to see if there are any exceptions related to threading when executing your program). Now there are some methods that do not throw an exception but for which the docs state they are not thread safe. It means that you wont necesarily have a crash, but that the program will behave in an unpredictable manner (and hence strange bugs).

    However when starting a thread you dont have to worry about unity API since you are not using it yet But you have to worry about a good way of starting your threads, This part is covered by MSDN / Unity / c# official docs, which you should generally use as a reference instead of users docs and exemples. Then will come a time where you will be able to discuss with the official guys who write the offical docs and point errors, but that's not for today
     
    Last edited: Nov 28, 2021
  7. Poo2thegeek

    Poo2thegeek

    Joined:
    Aug 30, 2014
    Posts:
    3
    I'm not calling the unity API. I understand what you mean now (I get this sometimes if i try to access the transform from a thread).
    This bug of a thread not starting after Thread.start is called has nothing to do with Unity directly, it's a c# bug of some sorts.
    The following is the "Generate" method, which takes a chunk as a parameter (the first in the list of toGenerate chunks)
    Code (CSharp):
    1. private void Generate(Chunk chunk){
    2.         Thread.Sleep (2);
    3.  
    4.         chunk.HeightMap = new int[Chunk.ChunkSize,Chunk.ChunkSize];
    5.         for (int x=0; x<Chunk.ChunkSize; x++) {
    6.             for (int z=0; z<Chunk.ChunkSize; z++) {
    7.                 //chunk.HeightMap[x,z] = HeightAt(x,z);
    8.                 chunk.HeightMap[x,z] = HeightAt(chunk.AbsoluteX + x, chunk.AbsoluteZ + z);
    9.  
    10.             }  
    11.         }
    12.         chunk.NonTerrainBlocks = new List<Block> ();
    13.         chunk.Generated = true;
    14.         chunk.Generating = false;
    15.  
    16.         chunk.Changed_ = true;
    17.         chunk.SaveIfNeeded_ = true;
    18.     }
    As you can see, no unity methods are called. All that happens is some of the chunk variables are changed.
    This is why I said this error is to do with C# probably.

    One idea I had was that if I remove the chunk before the Generate method is done, then this may terminate the Thread. I am however, not sure. I do not know how variables work between methods, and it seems a bit weird for me at the moment. I am used to java code where a variable is always a pointer, and a bit to c++ code where a variable is never a pointer unless you tell it to be.
    If this is the case then I believe the best course of action would be to add an extra boolean variable to my chunks called "IsBeingGenerated" (or something to that effect) which gets set to true while the Thread is running, and then iterate up through the chunks till I find one that is no longer being generated (chunk.IsBeingGenerated == false), and then start generating that. Only removing the chunk when I know it's being run.
    I could do this quicker if I created a class that is an instance of Thread, but apparently "Thread" is a closed class or something. Anyway of getting around this?