0

I created a method which handles a few checks before starting a Task passed as a parameter.

My issue is that the task created there do not behave as expected and are quickly considered as RanToCompletion despite code is still running.

Here is an example:

    public Task Main(CancellationToken localToken)
    {
        try
        {
            AddToTasker(new Task(async () => await ExtractAllOffer(localToken), localToken), TaskTypes.Extractor);

            //this allows to extract only the task for the given task type through the list created in AddToTasker, actual code is not necessary the returned array is correct
            Task.WaitAll(GetTasksArray(new TaskTypes[]{TaskTypes.Extractor}), localToken);

            IsRunning = false;
        }
    }

    public void AddToTasker(Task task, TaskTypes taskstype)
    {

        /*
         * Whatever code to perform few check before starting the task
         * among which referencing the task within a list which holds also the taskstype
         */


        task.Start();

    }

    async private Task ExtractAllOffer(CancellationToken localToken)
    {
        // *** Do very long Stuff ***
    }

The ExtractAllOffer method is a loop, with a few moment where i await external code completion. At the first await the Task.WaiAll terminates and it goes to IsRunning = false

I checked this thread but it does not seem to be the same issue as I correctly use an async task and not an async void.

Also, the code use to run properly before I shifted the Task execution within the AddToTasker method. Before I used to do like this AddToTasker(Task.Run(() => ExtractAllOffer(localToken), localToken), TaskTypes.Extractor); but I realized I needed to perform checks before starting the Task and needed to factor the checks within the AddToTasker method (I have many calls to this method).

I kind of understand that there is a flaw in how I declare or start my Task but can't figure what.

Help greatly appreciated

samuel guedon
  • 575
  • 1
  • 7
  • 21
  • 1
    The `Task` constructor and `Task.Run` do behave differently. See this [answer](https://stackoverflow.com/a/52769313/8910373). It's the same problem as the link you checked but perhaps the explanation is clearer. – pere57 Oct 21 '18 at 21:02
  • Thanks for the link, it helped a lot. I kind of understood what was gong on but did not know how to go around, I put the proper answer – samuel guedon Oct 21 '18 at 22:11

1 Answers1

1

Thanks to @pere57 and this other thread I see that the wait only wait until the action that creates the Task finishes... which is quite fast.

I have to declare it as a Task< Task> in order to the unwrap the first Task (the action) to access the inner Task (the actual method that gets executed).

So here it goes :

public Task Main(CancellationToken localToken)
{
    try
    {
        AddToTasker(new Task<Task>(() => ExtractAllOffer(localToken), localToken), TaskTypes.Extractor);

        //this allows to extract only the task for the given task type through the list created in AddToTasker, actual code is not necessary the returned array is correct
        Task.WaitAll(GetTasksArray(new TaskTypes[]{TaskTypes.Extractor}), localToken);

        IsRunning = false;
    }
}

public void AddToTasker(Task<Task> task, TaskTypes taskstype)
{

    /*
     * Whatever code to perform few check before starting the task
     * among which referencing the task within a list which holds also the taskstype
     */

    mylistoftask.Add(task.Unwrap()); //I have now to unwrap the task to add the inner one in my list
    task.Start();

}

async private Task ExtractAllOffer(CancellationToken localToken)
{
    // *** Do very long Stuff ***
}
samuel guedon
  • 575
  • 1
  • 7
  • 21
  • 1
    Why are you using `Task.Start` in the first place? There's no reason to use cold tasks. Just use Task.Run. There's no reason to use `Task` either. that's what `await` typically does. This code is over-complicated. If you want to perform checks, the parameter you pass should be the `Action` to execute, not a Task representing its execution. – Panagiotis Kanavos Oct 22 '18 at 15:20
  • 1
    What is the *actual* problem you want to solve? There may be a built-in solution already, eg `ActionBlock` can queue and process messages in a separate task already - no need to implement one's own worker class. Other classes in the Dataflow namespace can be used to create an entire processing pipeline. `await Task.WhenAll()` can await multiple tasks without blocking – Panagiotis Kanavos Oct 22 '18 at 15:24
  • To make it simple the AddToTasker method is a generic method which performs a few checks on task creation, among which to check if the perimeter is authorized, how many tasks per perimeters are running, ... Some Tasks uses managed code which get unstable above 40 concurrent tasks and sucks a lot of resources from my system. It seemed too heavy to call the checking methods prior to a Task.Run. For the record I was actually doing it before and it was a mess. My own worker also adds some audit/log data for debugging purpose. – samuel guedon Oct 22 '18 at 21:13
  • If the previous code was a mess, it wasn't `Task.Run` at fault. It means the validation code wasn't factored properly. Most likely it needed to be split into separate methods. Or replaced in its entirety with .NET's own validation mechanisms. Using cold tasks with all that additional code makes debugging a *lot* harder - that's why that `Unwrap` was needed. It doesn't solve the validation problem either, it covers it up with another layer of code - that doesn't work well either. – Panagiotis Kanavos Oct 23 '18 at 06:44
  • I disagree with what you say: "all that additional code" is deeply exaggerating as I traded all my calls to the validation methods prior to Task.Run for one line of code, the unwrap. So I actually factored my code like this, made the task creation easier to implement all with less lines of code. So yes it solves the problem and no it does not cover it with layers of code. Now regarding the use of Task.Run I agree that it should be favored as much as possible. – samuel guedon Oct 23 '18 at 07:04
  • Compared to eg `block.Post(someMessage);`? Or `Parallel.ForEach`? Or even a `Validate(payload); var results=await Tasl.Run(()=>ExtractAllOffer(payoload));` ? Perhaps you are using global/statics/field-level state when you should be passing parameters aroung? – Panagiotis Kanavos Oct 23 '18 at 10:46
  • No global, static, or field level here. Anyway, I am sure that there may be a better way to do, but I am a pragmatic : it works. And don't get it wrong, that is not a wish not to learn/find, but I am a part time programmer and my part time is scarce so I spend it on what's in my backlog. – samuel guedon Oct 23 '18 at 18:02