4

I'm using the tasks part of the Reactive Extensions library on .Net 3.5

It works mostly smoothly but in one place it calls the same task twice.

The call looks like this:

Task.Factory.StartNew(
    () => Processor.ProcessMessage(incomingMessage),
    TaskCreationOptions.PreferFairness );

Any ideas? Is this a bug?

---- update

i think the problem is with the way c# does closure in lambdas. the problem wasn"t in the TPL, the same problem returned with the plain old thread pool.

and this solved it:

foreach (var Processor in processors)
{
 object[] crap = new object[2];
 crap[0] = Processor;
 crap[1] = incomingMessage;
 Task.Factory.StartNew(Magic, crap, TaskCreationOptions.PreferFairness);
}

public void Magic(object obj)
{
 object[] crap =(object[]) obj;
 ((IIncomingMessageProcessor)crap[0]).ProcessMessage((IncomingMessageBase)crap[1]);
}

the original source was:

foreach (var Processor in processors)
{
Task.Factory.StartNew(
    () => Processor.ProcessMessage(incomingMessage),
    TaskCreationOptions.PreferFairness );
}

so i have closure around Processor, and i guess the problem was it was recycling the same object for the lambda and swapping the Processor.

---- update 2

I'm convinced this is the problem. i refactored and debugged the System.Threading.dll both times i create the task, it is created with the same delegate(Same ObjectID) and the Processor changes in the Target property between iterations. anyone knows a good work around?

---- update 3 this works too(thanks Judah Himango):

foreach (var processor in processors)
{
 var crap = processor;
 Task.Factory.StartNew(() => crap.ProcessMessage(incomingMessage), TaskCreationOptions.PreferFairness);
}
AK_
  • 7,981
  • 7
  • 46
  • 78
  • 1
    Please try to create a short but complete program which demonstrates the problem. If you could also see whether it occurs on .NET 4 (which has the TPL baked into the framework itself) that would be handy. – Jon Skeet Dec 07 '10 at 15:25
  • 1
    Perhaps Processor.ProcessMessage starts a task as well? – dtb Dec 07 '10 at 15:25
  • @dtb internaly it does. but the method ProcessMessage is still being called twice. @Jon i doubt i can because it mostly works fine... – AK_ Dec 07 '10 at 15:36
  • im Debuging it after i disassembled the code with Reflector. from what i see the "public Task StartNew(Action action, TaskCreationOptions creationOptions)" is being called twice – AK_ Dec 07 '10 at 16:46
  • it behaves the same with ThreadPool.QueueUserWorkItem – AK_ Dec 07 '10 at 19:08

1 Answers1

5

Are you using loop variables inside a lambda? e.g.

foreach (var foo in blah)
{
   // You're capturing foo inside a lambda -- this probably won't do what you think it will!
   Task.Factory.StartNew(() => foo.Something()); 
}

See Why is it bad to use an iteration variable in a lambda expression?

UPDATE November 2012: The new C# 5 compiler addresses this area of confusion by changing the behavior such that the foreach loop variable is captured, i.e. the expected behavior. Eric Lippert of the C# team writes,

We are taking the breaking change. In C# 5, the loop variable of a foreach will be logically inside the loop, and therefore closures will close over a fresh copy of the variable each time. The "for" loop will not be changed.

Community
  • 1
  • 1
Judah Gabriel Himango
  • 58,906
  • 38
  • 158
  • 212
  • thank you, ill read the articles in the link. but this is still a problem with the C# compiler in my opinion. – AK_ Dec 07 '10 at 20:01
  • It is a *known* problem however, and unlikely to be fixed, so you should learn to live with it. ReSharper for one warns about this. – Lasse V. Karlsen Dec 07 '10 at 20:09
  • It's actually the way lambdas were designed to work. Eric Lippert, one of the developers on the C# compiler team, blogged about this, and wrote about the problems with changing the behavior of the compiler, or offering a compiler warning: http://blogs.msdn.com/b/ericlippert/archive/2009/11/12/closing-over-the-loop-variable-considered-harmful.aspx – Judah Gabriel Himango Dec 07 '10 at 20:22
  • 1
    That said, I'm with you, I think in 99% of the cases, this is an accidental bug rising from developer misunderstanding. I suggest the C# compiler team modify the compiler to issue a warning when this is detected. The 1% that need this behavior can use a #pragma warning disable. – Judah Gabriel Himango Dec 07 '10 at 20:23
  • I'm glad I found this question. I had the exact same problem, and the code was pretty identical. We got around the issue by changing the call to ... Task.Factory.StartNew(foo.Something); – joshgo Nov 13 '12 at 17:48
  • For what it's worth, this area of confusion has been addressed in C# 5. The C# 5 compiler now captures the variable of a foreach loop, doing what you'd expect. See http://blogs.msdn.com/b/ericlippert/archive/2009/11/12/closing-over-the-loop-variable-considered-harmful.aspx – Judah Gabriel Himango Nov 13 '12 at 20:03