Is it bad practice to recursively call a thread method (System.Threading)?

Question

Forgive my terminology, I'm not all that familiar with System.Threading, but if I have something like the below:

private static int _index;
private static List<int> _movieIds = new List<int>();

static void Main(string[] args)
{
    // the below call populates the _movieIds list variable with around 130,000 ints
    GetListOfMovieIdsFromDatabase(); 

    _index = 0;
    Thread myThread = new Thread(DoWork);
    myThread.Start();
}

public static void DoWork()
{
     // do something with the value of _index (iterate through the _movieIds list) then recursively call DoWork() again

     Thread.Sleep(400);

     _index++;

     DoWork();
}

Is this bad practice? I am iterating through a private static list of int's defined at the class level as a member, so the first iteration of DoWork() would use the first value of _index (in a way I didn't explain for the sake of simplicity), then the second iteration (the recursive call) would work with the second value _index, and so on.

The reason why I'm asking this is because I'm getting a stack overflow exception after around 12 hours of running this app, and I believe it's because of the recursive calls.

Have you looked this question? http://stackoverflow.com/questions/145312/maximum-number-of-threads-in-a-net-app — , Oct 12 '13 at 23:25
If you have not a stop condition, the program will always crash because it's recursive. Does `DoWork` method really not receive any parameters ? If so, I think you should not use recursivity. — Julián Chamalé, Oct 12 '13 at 23:25

score 5 · Accepted Answer · edited Nov 24 '13 at 18:53

5

Yes. You will always eventually have a stack overflow because the calling stack never has a chance to unroll.

Instead of incrementing your index variable via recursive calls, use a loop in your thread.

public static void DoWork()
{
     while(true)
     {    
          // do something with the value of _index (iterate through the _movieIds list) then recursively call DoWork() again

         Thread.Sleep(400);

         _index++;

         // break when a condition is met   
      }
}

edited Nov 24 '13 at 18:53

Will Dean

39,055
11
90
118

answered Oct 12 '13 at 23:23

AndyG

39,700
8
109
143

`while(1)` is illegal. – Kirk Woll Oct 13 '13 at 00:17
@KirkWoll: My original comment did say `while(true)` but Black Frog changed it in his edit. – AndyG Oct 13 '13 at 21:42

score 2 · Answer 2 · answered Oct 12 '13 at 23:25

Yes, because the thread never exits. You need to provide an exit condition in order for the stack to unwind.

I assume you are trying to call multiple methods in parallel, which recursion does not do. It will still call the method serially, with the benefit that the results of one run can be used in the next. Since DoWork has no results, recursion in pointless here.

I see no reason for you to call this method recursively. At worst you could call DoWork in a simple loop that increments _index. You could try using Parallel.For to do the work in parallel to improve performance.

score 1 · Answer 3 · answered Nov 24 '13 at 18:09

You should look for the concept of tail recursion.

By implementing tail recursion you don't consume another stack each time you make a recursive call.

The concept itself explains why http://blogs.msdn.com/b/chrsmith/archive/2008/08/07/understanding-tail-recursion.aspx

Is it bad practice to recursively call a thread method (System.Threading)?

3 Answers3