CODEWITHSUNDEEP
c#.nettask-parallel-library
David Marchelya
David Marchelya

Reputation: 3432

TPL Fire and Forget using a Separate Class

I'm trying to implement fire and forget functionality, using the Task Parallel Library. With an inline call to Task.Factory.StartNew, everything works as expected. However, I want to move the Task.Factory.StartNew call into a separate class so that I can add logging, error handling, etc, and potentially upgrade the code in the future as better threading classes, etc are added to the .NET Framework, without duplicating code.

Below is a unit test that I would expect to pass, but that does not. I would appreciate help trying to figure out how to make this work.

[TestFixture]
public class ThreadingServiceFixture
{
    public static bool methodFired = false;

    [Test]
    public void CanFireAndForgetWithThreadingService()
    {
        try
        {
            var service = new ThreadingService();

            service.FireAndForget(() => methodFired = true);

            var endTime = DateTime.Now.AddSeconds(1);

            while(DateTime.Now < endTime)
            {
                //wait
            }

            Assert.IsTrue(methodFired == true);
        }
        finally
        {
            methodFired = false;
        }       
    }

}

public class ThreadingService
{
    public Task FireAndForget(Action action)
    {
        return Task.Factory.StartNew(() => action);
    }
}

Upvotes: 2

Views: 801

Answers (4)

oleksii
oleksii

Reputation: 35905

You did not invoke the action in the ThreadingService

The code should read something like

public class ThreadingService
{
    public Task FireAndForget(Action action)
    {
        return Task.Factory.StartNew(() => action.Invoke());
    }
}

Additional note: testing state with a public field is evil. Think about repeatability, maintenance, running tests in different order. You should move bool methodFired inside the test. I would also assume there is a better technique to test this (but I am not sure which one).

Upvotes: 2

Vitaliy
Vitaliy

Reputation: 8206

Testing threaded code is hard. Basing your tests on timing is a bad idea, they may become non-deterministic and you might observe erratic behavior on you build server. Imagine a tests that sometime passes and sometimes doesn't!

Your code has a bug, since you are not actually invoking the action.

But consider this variation:

[Test]
[TimeOut(5000)]
public void CanFireAndForgetWithThreadingService()
{
   var service = new ThreadingService();
   ManualResetEvent mre = new ManualRestEvent(bool); // I never remember what is the default...

   service.FireAndForget(() => mre.Set() /*will release the test asynchroneously*/);
   mre.WaitOne(); // blocks, will timeout if FireAndForget does not fire the action.
}

Yes, we are still using timing. But the test the timeout will happen only if the code breaks! In all other scenarios, the test is absolutely predictable and takes a very short amount of time to execute, no waiting and praying for timing issues not to happen ;-)

Upvotes: 1

Marcelo De Zen
Marcelo De Zen

Reputation: 9497

If is "fire and forget" you don't need to return the Task from the FireAndForget method, because the caller could get that Task and cancel it (strictly speaking the caller would "remember" of the call).

If you want to invoke this method from many services that do not inherit from a common ThreadingService you can implement an extension method via an interface.

public interface IFireAndForget 
{
     // no member needed.
}

public static class FireAndForgetExtensions 
{
    public static void FireAndForget(this IFireAndForget obj, Action action) 
    {
         // pass the action, not a new lambda
         Task.Factory.StartNew(action);
    }
}


// using
public class ThreadingService : IFireAndForget 
{

}

Also note the in your method you have to pass the action to the StartNew method insted of pass a lambda that return the action parameter.

Upvotes: 2

Nick Butler
Nick Butler

Reputation: 24383

You're not executing the action, you're just returning it.

Try:

return Task.Factory.StartNew(() => action());

Upvotes: 3

Related Questions