How to create a thread/Task with a continuous loop?

Question

I am looking for the correct way/structure to create a loop in a Thread/Task...

The reason for this is, i need to check the DB every 15sec for report requests.

This is what i tried so far, but i get OutOfMemoryException:

    private void ViewBase_Loaded(object sender, RoutedEventArgs e)
{
    //On my main view loaded start thread to check report requests.
    Task.Factory.StartNew(() => CreateAndStartReportRequestTask());
}

private void CreateAndStartReportRequestTask()
{
    bool noRequest = false;

    do
    {
         //Starting thread to Check Report Requests And Generate Reports
         //Also need the ability to Wait/Sleep when there are noRequest.
         reportRequestTask = Task.Factory.StartNew(() => noRequest = CheckReportRequestsAndGenerateReports());

         if (noRequest)
         {
             //Sleep 15sec
             reportRequestTask.Wait(15000);
             reportRequestTask = null;
         }
         else
         {
             if (reportRequestTask.IsCompleted)
             {
                 reportRequestTask = null;
             }
             else
             {
                 //Don't want the loop to continue until the first request is done
                 //Reason for this is, losts of new threads being create in CheckReportRequestsAndGenerateReports()
                 //Looping until first request is done.
                 do
                 {

                 } while (!reportRequestTask.IsCompleted);

                 reportRequestTask = null;
             }
         }

    } while (true);
}

private bool CheckReportRequestsAndGenerateReports()
{
    var possibleReportRequest = //Some linq query to check for new requests

    if (possibleReportRequest != null)
    {
        //Processing report here - lots of new threads/task in here as well
        return false;
    }
    else
    {
        return true;
    }
}

What am i doing wrong?

Is this correct way or am i total off?

EDIT:

Most important, my UI must still be responsive!

Vhf

See https://mcmap.net/q/319575/-cpu-friendly-infinite-loop/… – Aubry

Instead of placing the loop inside of this task, you create the task in endless loop. – Dextrous

You're creating who knows how many Tasks. Your code doesn't make much sense. You probably should edit and describe exactly what you are trying to accomplish. – Carisa

Who tells your process when to end? The program? Or the task processing itself (i.e. once you get FALSE return)? – Offbeat

It's worth noting that best practice in 2021 is to use Task.Run not Task.Factory.StartNew – Digestif

Answer 1

Sounds like you want something like this. Please correct me if I am misinterpretting your intentions...

First, in your kick-off, set as a long running task so it doesn't consume a thread from the thread pool but creates a new one...

private void ViewBase_Loaded(object sender, RoutedEventArgs e)
{
    // store this references as a private member, call Cancel() on it if UI wants to stop
    _cancelationTokenSource = new CancellationTokenSource();
    new Task(() => CreateAndStartReportRequestTask(), _cancelationTokenSource.Token, TaskCreationOptions.LongRunning).Start();
}

Then, in your report watching thread, loop until IsCancelRequested has been set. If there is no work, just wait on the cancel token for 15 seconds (this way if cancelled will wake sooner).

private bool CheckReportRequestsAndGenerateReports()
{
    while (!_cancellationTokenSource.Token.IsCancelRequested) 
    {
        var possibleReportRequest = //Some linq query
        var reportRequestTask = Task.Factory.StartNew(() => noRequest = CheckReportRequestsAndGenerateReports(), _cancellationTokenSource.Token);

        if (noRequest)
        {
            // it looks like if no request, you want to sleep 15 seconds, right?
            // so we'll wait to see if cancelled in next 15 seconds.
            _cancellationTokenSource.Token.WaitHandle.WaitOne(15000);

        }
        else
        {
            // otherwise, you just want to wait till the task is completed, right?
            reportRequestTask.Wait(_cancellationTokenSource.Token);
        }
    }
}

I'd also be wary of having your task kick off more tasks. I have a feeling you are spinning up so many you're consuming too many resources. I think the main reason your program was failing was that you had:

     if (noRequest)
     {
         reportRequestTask.Wait(15000);
         reportRequestTask = null;
     }

This will return immediately and not wait 15s, because the thread is already complete at this point. Switching it to the cancel token (or a Thread.Sleep(), but then you can't abort it as easily) will give you the processing wait you need.

Hope this helps, let me know if i'm off on my assumptions.

Offbeat

Thanks James. This sounds about right. Just read my update quick. I added some comments. – Vhf

@Willem: I read your updates, I think I have my loop right in respect to them. This will basically kick off the report checker as a long-running task (thread) and inside, if there is no work created it will wait for 15s (or cancellation, whichever comes first) and then try again. If there IS work, it waits for that work to be done then repeats (until cancelled). – Offbeat

Thanks james. It seems to all work fine. One last problem. In CheckReportRequestsAndGenerateReports(), i have a linq query that needs to access the main thread. I call this.Dispatcher.Invoke((Action)(() => { //some linq query })); Works fine with the first request, but just stalls the app on the second request. Any idea what i am doing wrong? – Vhf

@Willem: Hmmm, would need to see the code, can you post a swatch of it in an update to your question? – Offbeat

@Willem: Ah great! What was the issue with the dispatch? – Offbeat

I used Dispatcher.Invoke. Changed it to Dispatcher.BeginInvoke. Solved my problem. =) – Vhf

Hands down fantastic explanation. – Gnash

This works great and is very adaptable - I was able to patch bits and pieces into my code and have everything run smoothly. Never would have got it to work so nicely without this. – Wormwood

Answer 2

Something like this would work:

var cancellationTokenSource = new CancellationTokenSource();
var task = Repeat.Interval(
        TimeSpan.FromSeconds(15),
        () => CheckDatabaseForNewReports(), cancellationTokenSource.Token);

The Repeat class looks like this:

internal static class Repeat
{
    public static Task Interval(
        TimeSpan pollInterval,
        Action action,
        CancellationToken token)
    {
        // We don't use Observable.Interval:
        // If we block, the values start bunching up behind each other.
        return Task.Factory.StartNew(
            () =>
            {
                for (;;)
                {
                    if (token.WaitCancellationRequested(pollInterval))
                        break;

                    action();
                }
            }, token, TaskCreationOptions.LongRunning, TaskScheduler.Default);
    }
}

static class CancellationTokenExtensions
{
    public static bool WaitCancellationRequested(
        this CancellationToken token,
        TimeSpan timeout)
    {
        return token.WaitHandle.WaitOne(timeout);
    }
}

Rhetor

@Roger: I like your Repeat class, I wonder if you should add an option to create the task as TaskCreationOptions.LongRunningTask to avoid hogging a thread in the threadpool – Offbeat

@Roger: Mind if I add your class to our bag of tricks here where I work? We have a very robust Shared Components Committee where we try to make common, reusable utility classes, and this one is wonderfully generic (in the best sense of the term). – Offbeat

@RogerLipscombe Sheer curiosity: how come the for(;;) insated of a while(true)? – Formaldehyde

@Formaldehyde Holdover from my C/C++ days, when while(true) would often give a warning -- "condition is always constant" or something like that. I think. – Rhetor

...also, I find it easier to read. In while(true), you have to actually look at the word 'true'. In for (;;), the whole thing reads as "forever". – Rhetor

@RogerLipscombe I've been using your class above but I don't understand how if (token.WaitCancellationRequested(pollInterval)) break; works. From what I can fathom is that once the timeout expires it will return true and then break out of the loop and never execute again, can you provide a layman's guide? – Minh

Also @RogerLipscombe in terms of usage, if I wanted to block the host thread from continuing whilst Repeat.Interval() was executing should I use the returned task and call task.Wait(); or task.Wait(cancellationTokenSource.Token);? – Minh

Try-catch the action() call, in case of miscellaneous errors when you might want to retry the action. – Coaler

even just naming WaitOne as WaitCancellationRequested is great – Recalescence

Answer 3

I've made a work-around starting from @Roger's answer. (A friend of mine has given good advices regarding this too)... I copy it here I guess it could be useful:

/// <summary>
/// Recurrent Cancellable Task
/// </summary>
public static class RecurrentCancellableTask
{
    /// <summary>
    /// Starts a new task in a recurrent manner repeating it according to the polling interval.
    /// Whoever use this method should protect himself by surrounding critical code in the task 
    /// in a Try-Catch block.
    /// </summary>
    /// <param name="action">The action.</param>
    /// <param name="pollInterval">The poll interval.</param>
    /// <param name="token">The token.</param>
    /// <param name="taskCreationOptions">The task creation options</param>
    public static void StartNew(Action action, 
        TimeSpan pollInterval, 
        CancellationToken token, 
        TaskCreationOptions taskCreationOptions = TaskCreationOptions.None)
    {
        Task.Factory.StartNew(
            () =>
            {
                do
                {
                    try
                    {
                        action();
                        if (token.WaitHandle.WaitOne(pollInterval)) break;
                    }
                    catch
                    {
                        return;
                    }
                }
                while (true);
            },
            token,
            taskCreationOptions,
            TaskScheduler.Default);
    }
}

Dystrophy

For my current lightweight task, this is perfect. Thank you :) – Directly

don't you think when while is always true using do-while is ugly? – Recalescence

Answer 4

feeling adventurous?

internal class Program
{
    private static void Main(string[] args)
    {
        var ct = new CancellationTokenSource();

        new Task(() => Console.WriteLine("Running...")).Repeat(ct.Token, TimeSpan.FromSeconds(1));

        Console.WriteLine("Starting. Hit Enter to Stop.. ");
        Console.ReadLine();

        ct.Cancel();

        Console.WriteLine("Stopped. Hit Enter to exit.. ");
        Console.ReadLine();
    }
}


public static class TaskExtensions
{
    public static void Repeat(this Task taskToRepeat, CancellationToken cancellationToken, TimeSpan intervalTimeSpan)
    {
        var action = taskToRepeat
            .GetType()
            .GetField("m_action", BindingFlags.NonPublic | BindingFlags.Instance)
            .GetValue(taskToRepeat) as Action;

        Task.Factory.StartNew(() =>
        {
            while (true)
            {
                if (cancellationToken.WaitHandle.WaitOne(intervalTimeSpan))
                    break;
                if (cancellationToken.IsCancellationRequested)
                    break;
                Task.Factory.StartNew(action, cancellationToken);
            }
        }, cancellationToken);
    }
}

Simonson

I don't think this is a good idea. The big problem is that the task to repeat will not behave as if it was in a loop. With that code, multiple instances will be spawned in parallel, with no control on how many instances there is running at the same time. For the loop to work correctly each iteration must be started only once the previous one is completed. Also, the while(true) is consuming a thread from the ThreadPool that is essentially standing there waiting to start a new task. – Timmi

there's no requirement for pipelining. OP just wants to run a task every X seconds :) – Simonson