-1

Consider the snippet bellow.
A form adds a BusyForm to the queue and later on, when the form is done, it sets the result for the TCS, marking it as complete, without removing the item from the queue.
After a while, something happens and we need to wait for all busy forms, so we await each TCS task in the queue. But surprise, surprise .. GC grabbed it.

Why? Isn't it referenced by the queue?

static class Global
{
    public static ConcurrentQueue<BusyForm> BusyForms = new ConcurrentQueue<BusyForm>();

    public class BusyForm
    {
        public string Name;
        public TaskCompletionSource<bool> Done = new TaskCompletionSource<bool>();

        public BusyForm(string name)
        {
            this.Name = name;
        }
    }
}
// somewhere else when form is done
busyForm.Done.SetResult(true);
// after a while, when required
while(!Global.BusyForms.IsEmpty)
{
    Global.BusyForms.TryPeek(out busyForm);
    await busyForm.Done.Task; // can be disposed by GC if finished a long time ago
    Global.BusyForms.TryDequeue(out busyForm);
}
Chris
  • 1,213
  • 1
  • 21
  • 38
  • What do you mean by "GC grabbed it" (and how do you know that)? Are you talking about `busyform` or `busyForm.Done` or `busyform.Done.Task`? – D Stanley Apr 09 '15 at 14:23
  • 1
    Can be create a small but *complete* example that demonstrates the problem? Also, the use of `disposed` when talking about GC may indicate some confusion here about two separate concepts, so it would help to know exactly how you know that the object has been collected/disposed. An error? Something else? – Damien_The_Unbeliever Apr 09 '15 at 14:23
  • @DStanley I get an object disposed exception when I await "busyForm.Done.Task" task – Chris Apr 09 '15 at 14:24
  • And why don't you just dequeue it rather than leaving in on the queue? Seems like that would set up race conditions if the same task were processed twice. – D Stanley Apr 09 '15 at 14:25
  • 2
    GC does not dispose objects - either you're explicitly calling `Dispose` or you've got it in a `using` block. – D Stanley Apr 09 '15 at 14:26
  • @DStanley I'm not calling Dispose/using. I can't remove it from the queue when form is done, because it might be await'ed already. – Chris Apr 09 '15 at 14:31
  • A `Task` is disposable, neither your `BusyForm` nor a `TaskCompletionSource` is. So if you're getting an `ObjectDisposedException`, something has `Dispose`d the `Task` that the `TaskCompletionSource` returns - either explicitly or, as D Stanley says, via putting it in a `using` statement. – Damien_The_Unbeliever Apr 09 '15 at 14:32
  • @Damien_The_Unbeliever So I have to keep a reference for the returned Task by the TCS as well. How do I do that before it's completed? – Chris Apr 09 '15 at 14:35
  • @Chris Then perhaps the _task_ is disposing of something that it had a reference to? Are you certain no task is getting executed twice? It seems odd to process something _while it's on the queue_ - typically items are removed from the queue and processed out-of band to prevent the same item from being processed twice (and to let the next item be processed). But you haven't given enough context to know if there's a valid reason for leaving it on the queue. – D Stanley Apr 09 '15 at 14:36
  • 1
    @Chris - NO. This has nothing to do with references or garbage collection, and everything to do with the Disposable pattern. Stop thinking about garbage collection because it's *not* involved here at all. – Damien_The_Unbeliever Apr 09 '15 at 14:41
  • @Chris If the task has not yet completed then _something_ has a reference to it. I'm with Damien - this has nothing to do with GC and everything to do with re-using a disposed object. You just need to figure out which object is disposed, why, and how to prevent it. – D Stanley Apr 09 '15 at 14:44
  • I doubt it is guaranteed that TryPeek and TryDequeue result in the same object being returned. Why call the queue twice? Just call TryDequeue and use the return value. – usr Apr 09 '15 at 14:47

1 Answers1

1

It's not clear how this all fits together, but it seems like your process should just be:

while(!Global.BusyForms.IsEmpty)
{
    if(Global.BusyForms.TryDequeue(out busyForm))
        await busyForm.Done.Task; // can be disposed by GC if finished a long time ago
    else
    {
         // could not dequeue for some reason - need to break the loop so that we no not get in an infinite loop.  
         break;
         // or perhaps throw an exception?
    }
}

That way there is no risk of the same Task being processed twice. It seems odd to process an item while it's still on the queue. Think about checking in at the airport - you don't check in while you're in line - you get pulled out of line to be served by the next available agent.

D Stanley
  • 149,601
  • 11
  • 178
  • 240