3

I'd like some clarification on code I didn't do and have to modify in a service.

Here's some parts of the code of the service

private Thread _thread;
private ConcurrentQueue<Task> _tasks = new ConcurrentQueue<Task>();
private Task _runningTask = null;

protected override void OnStart(string[] args)
{
    _thread = new Thread(WorkerThreadFunc);
    _thread.IsBackground = true;
    _thread.Start();
}

private void WorkerThreadFunc()
{
    InitDb();

    while (!_shutdownEvent.WaitOne(1000))
    {
        if (_runningTask == null || _runningTask.IsCompleted)
        {
            Task task;
            if (_tasks.TryDequeue(out task))
            {
                _runningTask = task;
                _runningTask.Start();
            }
        }
    }
}


private void RunReport(int reportID)
{
    var task = new Task(id =>
    {
        //Task code
    }, reportID);

    _tasks.Enqueue(task);
}

And so, this all works well

The thing is, I want to add other tasks to the task queue, but I don't have any ID to give them (the task in the code runs a report and uses the reportID, but the other tasks aren't linked to one report in particular).

Is there a way to create a Task without giving it an ID (which I doubt), or is there something i'm completely missing ?

Shadowxvii
  • 1,080
  • 2
  • 12
  • 31
  • I know this isn't what you're asking, but since I noticed: this code `while (!_shutdownEvent.WaitOne(1000))` looks suspect - it will cause your worker thread to idle for 1 second before taking on next task - seems waste of resources. using some sort of synchronization event to signal that there are items in the queue and then waiting for both, that event and for shutdown even would be a better way of using system resources. Again, this comment is outside the scope of your question - but since I noticed that, I thought I'd let you know... – LB2 Feb 26 '14 at 21:15
  • I'll admit that I've been thinking that there might be something wrong with that line, but just couldn't figure out how to change it since i'm not really familiar with threads and tasks. – Shadowxvii Feb 26 '14 at 21:19
  • Good point, I added the variable definition needed for the code snippet – Shadowxvii Feb 26 '14 at 21:26
  • @Shadowxvii If you switched to a `BlockingCollection` instead of `ConcurrentQueue`, you could replace the while loop with GetConsumingEnumerable() and a foreach... Would avoid the delay in processing. – Reed Copsey Feb 26 '14 at 21:28
  • ok, that queue type is concurrency friendly (as name suggests). So deleted my other comment – LB2 Feb 26 '14 at 21:28

2 Answers2

3

Is there a way to create a Task without giving it an ID (which I doubt), or is there something i'm completely missing ?

Yes, just use:

_tasks.Enqueue(new Task(() => {
       // Your code here
   });

That being said, this is not an idiomatic usage of the Task class. In general, the Task and Task<T> classes are meant to always be running - not created and started later.

Reed Copsey
  • 554,122
  • 78
  • 1,158
  • 1,373
  • I'm sure that it's using the default Task class (not a custom made one). I looked a bt before asking the question, and that's because I couldn't find Tasks being used the way it is in my code that I had to ask. I'll just putting nothing as an ID and see how this works. Thanks. – Shadowxvii Feb 26 '14 at 21:18
  • I looked at the definition for Task and this is what I got from the metadata : `public Task(Action action, object state);` – Shadowxvii Feb 26 '14 at 21:31
  • @Shadowxvii Ahh, okay - yes, you're passing it as state - nevermind - you can just use the Task from my first version, then - will edit – Reed Copsey Feb 26 '14 at 21:31
2

I know it's not your code to begin with but, like Reed said, you shouldn't queue Tasks. You should queue work items instead.

Take a look at TPL DataFlow. There's even an introductory document.

Community
  • 1
  • 1
Paulo Morgado
  • 14,111
  • 3
  • 31
  • 59