C#•7mo ago

Help with Multithreading

Hi, I'm currently trying to simulate an elevator system. Since this is a system of elevators that work in parallel, I found that multithreading might be the best fit for it. I'm very new to multithreading in C# and this bit of code gives me an OutOfBound Exception. Context: so currently I have an elevator systems of n elevators, and each elevators has its own Move method where it would operate the elevators independently. Currently I want to assign each elevator to a thread to run alongside with my main thread. So I have the idea of using an array of threads with the same size as the elevators array in the system. Here I thought when assigning each elevators to each threads would be as simple as using a for loop. However, after assign a new thread to each threads, I try to start each thread sperately and keep getting an OutOfBound Exception. During debugging, the assigning bit was fine and all. The main problem was starting the very first thread in the thread array. It keeps saying that i = 4, and the elevators array index was only up to 3. But the thing is I don't understand how i could be 4? Or if i even exists out of the first for loop scope? I have absolutely no clue at this point because it seems like it was using i as a reference and not a value?

19 Replies

waler•7mo ago

I can of course create each threads manually, but I'm trying to generalise this for case with any amounts of elevators. I would appreciate any other idea recommendations as well.

Jorge Morales•6mo ago

Hi, I think it's because i = 4 after the for loop is done. Besides, inside of each ThreadStart you have a reference to i, which the first thread is going to use and, because i is 4, then you get the exception you saw. You would have to rewrite your code inside of ThreadStart to not depend on i Maybe I'm wrong, as I don't have enough information besides your code snippet, but I guess it has something to do with i

cap5lut•6mo ago

Jorge Morales is correct, u would have to use an in-loop extra variable so it captures correctly

for (int i = 0; i < test.elevators.Length; i++)
{
  int elevatorIndex = i;
  threads[i] = new Thread(() => {
    while (!Console.KeyAvailable)
      test.elevators[elevatorIndex].Move(test);
  });
}

for (int i = 0; i < test.elevators.Length; i++)
{
  int elevatorIndex = i;
  threads[i] = new Thread(() => {
    while (!Console.KeyAvailable)
      test.elevators[elevatorIndex].Move(test);
  });
}

because the elevatorIndex is inside the loop, there exist a different variable for each iteration. i exists outside of the iteration but only for the scope of the loop, so with the original code it would capture the very same variable each time. (as a side note: u do not need to call new ThreadStart(), the lambda u pass to that constructor can also be directly be translated to the ThreadStart delegate by the compiler) u can observe this if u run this simple program a couple of times

Thread[] threads = new Thread[4];
for (int i = 0; i < threads.Length; i++)
{
    var elevatorId = i;
    threads[i] = new Thread(() => Console.WriteLine($"captured i={i}, elevatorId={elevatorId}"));
    threads[i].Start();
}
foreach (var thread in threads) thread.Join();

Thread[] threads = new Thread[4];
for (int i = 0; i < threads.Length; i++)
{
    var elevatorId = i;
    threads[i] = new Thread(() => Console.WriteLine($"captured i={i}, elevatorId={elevatorId}"));
    threads[i].Start();
}
foreach (var thread in threads) thread.Join();

u will see some output like

captured i=2, elevatorId=0
captured i=2, elevatorId=1
captured i=2, elevatorId=2
captured i=4, elevatorId=3

captured i=2, elevatorId=0
captured i=2, elevatorId=1
captured i=2, elevatorId=2
captured i=4, elevatorId=3

where i will be completely random each time and elevatorId will always be 0, 1, 2 and 3 (note that this can possibly be in another order as well, but it will be always this set of numbers)

waler•6mo ago

hmm I get a hint that it was using i as a reference too. Although may I ask why is this the case? It's so weird to see a value type be passed by reference even though I didn't specifically ask it to pass by ref also thank you so much for the fix For the elevatorindex, that was actually a really nice way to get over the pass by reference bit. Also did not know I could avoid constructing a new ThreadStart every iteration.

cap5lut•6mo ago

its not used as ref, the compiler transforms the code quite a lot if u look at the lowered C# code:

MODiX•6mo ago

cap5lut

sharplab.io (click here)

Thread[] threads = new Thread[4];
for (int i = 0; i < threads.Length; i++) {
    var elevatorId = i;
    threads[i] = new Thread(() => Console.WriteLine($"capt...
    threads[i].Start();
}
foreach (var thread in threads) thread.Join();

Thread[] threads = new Thread[4];
for (int i = 0; i < threads.Length; i++) {
    var elevatorId = i;
    threads[i] = new Thread(() => Console.WriteLine($"capt...
    threads[i].Start();
}
foreach (var thread in threads) thread.Join();

React with ❌ to remove this embed.

cap5lut•6mo ago

the names are weird and not allowed in C# to avoid naming collisions, i marked all relevant code to capturing i with // here and the capture of elevatorId with // also here

        Thread[] array = new Thread[4];
        <>c__DisplayClass0_0 <>c__DisplayClass0_ = new <>c__DisplayClass0_0();
        <>c__DisplayClass0_.i = 0; // here
        while (<>c__DisplayClass0_.i < array.Length)
        {
            <>c__DisplayClass0_1 <>c__DisplayClass0_2 = new <>c__DisplayClass0_1();
            <>c__DisplayClass0_2.CS$<>8__locals1 = <>c__DisplayClass0_; // here
            <>c__DisplayClass0_2.elevatorId = <>c__DisplayClass0_2.CS$<>8__locals1.i; // also here
            array[<>c__DisplayClass0_2.CS$<>8__locals1.i] = new Thread(new ThreadStart(<>c__DisplayClass0_2.<<Main>$>b__0));
            array[<>c__DisplayClass0_2.CS$<>8__locals1.i].Start();
            <>c__DisplayClass0_.i++; // here
        }

        Thread[] array = new Thread[4];
        <>c__DisplayClass0_0 <>c__DisplayClass0_ = new <>c__DisplayClass0_0();
        <>c__DisplayClass0_.i = 0; // here
        while (<>c__DisplayClass0_.i < array.Length)
        {
            <>c__DisplayClass0_1 <>c__DisplayClass0_2 = new <>c__DisplayClass0_1();
            <>c__DisplayClass0_2.CS$<>8__locals1 = <>c__DisplayClass0_; // here
            <>c__DisplayClass0_2.elevatorId = <>c__DisplayClass0_2.CS$<>8__locals1.i; // also here
            array[<>c__DisplayClass0_2.CS$<>8__locals1.i] = new Thread(new ThreadStart(<>c__DisplayClass0_2.<<Main>$>b__0));
            array[<>c__DisplayClass0_2.CS$<>8__locals1.i].Start();
            <>c__DisplayClass0_.i++; // here
        }

basically, i doesnt exist anymore, <>c__DisplayClass0_.i is the new i. <>c__DisplayClass0_ is shared by all threads and its a question when they actually print which state the i member has, which is a race condition. <>c__DisplayClass0_2.elevatorId is the capured elevatorId, each thread has its own instance of this, thus there is no race condition well, as u can see in the lowered code, new ThreadStart() will still be there, its just less boilerplate code for u to write, the compiler does generates that for u. in the lowered code u can also observe, that there isnt any lambda expression anymore, they simply dont exist when compiled, the compiler generates a class with a method for it so for example for this code

public static Program
{
  public static void Main()
  {
    Action action = () => Console.WriteLine();
  }
}

public static Program
{
  public static void Main()
  {
    Action action = () => Console.WriteLine();
  }
}

the compiler generates something like

public static Program
{
  public static void Main()
  {
    Action action = new Action(CompilerGeneratedClassForTheLambdaExpression.instance.TheLambdaExpression);
  }

  private sealed class CompilerGeneratedClassForTheLambdaExpression
  {
    internal static CompilerGeneratedClassForTheLambdaExpression instance = new CompilerGeneratedClassForTheLambdaExpression();

    internal void TheLambdaExpression()
    {
      Console.WriteLine();
    }
  }
}

public static Program
{
  public static void Main()
  {
    Action action = new Action(CompilerGeneratedClassForTheLambdaExpression.instance.TheLambdaExpression);
  }

  private sealed class CompilerGeneratedClassForTheLambdaExpression
  {
    internal static CompilerGeneratedClassForTheLambdaExpression instance = new CompilerGeneratedClassForTheLambdaExpression();

    internal void TheLambdaExpression()
    {
      Console.WriteLine();
    }
  }
}

waler•6mo ago

hmmm I see, so the real reason was because of a race condition that was bound to happen, if I maybe did something like making the main thread sleep for a bit after assign a new thread to the threads array, can this avoid the race condition potentially happening?

cap5lut•6mo ago

yes and no, basically u have to sleep until the thread executes and uses the variable. eg, this code would print for all threads that i is 4

Thread[] threads = new Thread[4];
for (int i = 0; i < threads.Length; i++)
{
    var elevatorId = i;
    threads[i] = new Thread(() => Console.WriteLine($"captured i={i}, elevatorId={elevatorId}"));
}
Thread.Sleep(1000);
foreach (var thread in threads) thread.Start();
foreach (var thread in threads) thread.Join();

Thread[] threads = new Thread[4];
for (int i = 0; i < threads.Length; i++)
{
    var elevatorId = i;
    threads[i] = new Thread(() => Console.WriteLine($"captured i={i}, elevatorId={elevatorId}"));
}
Thread.Sleep(1000);
foreach (var thread in threads) thread.Start();
foreach (var thread in threads) thread.Join();

waler•6mo ago

hmmm I see wow timing is really a headaching thing with multithreading

cap5lut•6mo ago

thats why u do that local copy of i to have a different capture

waler•6mo ago

yeah, seems like that's the best way there is I have also heard of a locking mechanism? Though I do wonder if that helps with anything here

cap5lut•6mo ago

basically as soon as multiple threads access the same state u have to be really careful if the thread's lambda would be

() => {
  Console.WriteLine(i);
  Thread.Sleep(1000);
  Console.WriteLine(i);
}

() => {
  Console.WriteLine(i);
  Thread.Sleep(1000);
  Console.WriteLine(i);
}

it could totally print 2 different values for i locking doesnt help help here, locking is simply to protect from simultaneous access

waler•6mo ago

oh right, because the loop keeps iterating ok wow, now this makes me worry since my system actually has one more logic where I would need to calculate the algorithm (possibly in the main thread) for choosing a destination floor while the elevator is moving. alright, thank you so much for the help. I think I'll go read more on this topic

cap5lut•6mo ago

locks are used for something like this:

public class Program
{
    public static int counter;

    public static void Main()
    {
        Semaphore semaphore = new Semaphore(0, 10);
        Thread[] threads = new Thread[10];
        for (int i = 0; i < threads.Length; i++)
        {
            threads[i] = new Thread(() =>
            {
                semaphore.WaitOne();
                for (int i = 0; i < 1000; i++)
                {
                    counter++;
                }
            });
        }
        foreach (var thread in threads) thread.Start();
        Thread.Sleep(100);
        semaphore.Release(10);

        foreach (var thread in threads) thread.Join();
        Console.WriteLine(counter);
    }
}

public class Program
{
    public static int counter;

    public static void Main()
    {
        Semaphore semaphore = new Semaphore(0, 10);
        Thread[] threads = new Thread[10];
        for (int i = 0; i < threads.Length; i++)
        {
            threads[i] = new Thread(() =>
            {
                semaphore.WaitOne();
                for (int i = 0; i < 1000; i++)
                {
                    counter++;
                }
            });
        }
        foreach (var thread in threads) thread.Start();
        Thread.Sleep(100);
        semaphore.Release(10);

        foreach (var thread in threads) thread.Join();
        Console.WriteLine(counter);
    }
}

(ignore the semaphore, i just use that so that the threads start executing their for loops at the exact same time) so basically each thread increments the counter 1000 times by one, u would assume that the counter is afterwards 10000 right? and thats a problem, because it most is something below 10000 (just ran it myself, and i got 7840 as result) that is because counter++ isnt an atomic operation, but actually 3 operations and thus is not thread safe counter++ consists of 3 operations 1) read the value of counter 2) increment the value by 1 3) write the value back to counter so if 2 threads read the value at the same time and they get back 0 as value, they will write back 1 as value and lets just hope the other threads didnt already raise the counter to 2 or so. this is where locking comes into play, to make it thread safe, u have to make sure only one thread at a time can read, increment and then write counter value this would always print 10000

public class Program
{
    private static object _lockObject = new object();
    public static int counter;

    public static void Main()
    {
        Semaphore semaphore = new Semaphore(0, 10);
        Thread[] threads = new Thread[10];
        for (int i = 0; i < threads.Length; i++)
        {
            threads[i] = new Thread(() =>
            {
                semaphore.WaitOne();
                for (int i = 0; i < 1000; i++)
                {
                    lock (_lockObject)
                    {
                      counter++;
                    }
                }
            });
        }
        foreach (var thread in threads) thread.Start();
        Thread.Sleep(100);
        semaphore.Release(10);

        foreach (var thread in threads) thread.Join();
        Console.WriteLine(counter);
    }
}

public class Program
{
    private static object _lockObject = new object();
    public static int counter;

    public static void Main()
    {
        Semaphore semaphore = new Semaphore(0, 10);
        Thread[] threads = new Thread[10];
        for (int i = 0; i < threads.Length; i++)
        {
            threads[i] = new Thread(() =>
            {
                semaphore.WaitOne();
                for (int i = 0; i < 1000; i++)
                {
                    lock (_lockObject)
                    {
                      counter++;
                    }
                }
            });
        }
        foreach (var thread in threads) thread.Start();
        Thread.Sleep(100);
        semaphore.Release(10);

        foreach (var thread in threads) thread.Join();
        Console.WriteLine(counter);
    }
}

the lock (_lockObject) lets a thread aquire the lock, if no other thread has the lock already. if another thread has already aquired the lock, the thread that wants the lock is in a suspended state, meaning it pauses execution until it can get the lock (if multiple threads are waiting, its undefined which thread will get the lock next) but for more about this u should read the documentation ;p

cap5lut•6mo ago

https://learn.microsoft.com/en-us/dotnet/standard/threading/managed-threading-basics

Managed Threading Basics - .NET

See links to other managed threading articles, covering topics such as exceptions, synchronizing data, foreground & background threads, local storage, and more.

waler•6mo ago

ok wow, thank you so much I'll read the document first and probably go back to the example you gave, all I understand as of now is there might be multiple threads that read and write the value of counter at the same time, which is why we did not get 10000 as a result.

cap5lut•6mo ago

so if ur questions are answered, please dont forget to $close the thread

MODiX•6mo ago

Use the /close command to mark a forum thread as answered

Gaming

Programming

Help with Multithreading