1

I'm writing a utility class for some asynchronous code, and I want to ensure that I don't create memory leaks with the design. Suppose I've got code which executes similarly to the class below. (Obviously you wouldn't write code that works like this, but I've got several code paths which all converge into a single underlying function, and one of those code paths is effectively doing this). The key point in the code is that nothing in my code holds a reference to either the TaskCompletionSource or its Task, though .NET might be doing some voodoo which does root them.

Suppose that Foo instances are created as needed and are not permanently rooted. Is tcs.Task rooted? More importantly, suppose request never completes, so the result doesn't get set. Would tcs.Task hang around forever? What about the continuation task? Or, is their lifetime dependent on the Foo object and they will go away when the Foo instance gets GC-ed?

class Foo
{
    public void Bar(Action<Action<object>> request, Action<T> callback)
    {
        var tcs = new TaskCompletionSource<object>();
        request(result => tcs.SetResult(result));  // this runs asynchronously
        tcs.Task.ContinueWith(task => callback(task.Result));
    }
}
aaronburro
  • 504
  • 6
  • 15
  • 4
    the lambda you pass to request (your code) holds a reference to tcs – Sir Rufo May 27 '20 at 04:48
  • If you're concerned about memory leaks, it's far more productive to use a memory profiler to find them. – Damien_The_Unbeliever May 27 '20 at 07:22
  • I been staring at this code way too long not to have seen that. Thanks, Rufo. Then again, nowhere was the code this explicit to see the obvious. If you want to make an answer, I'll mark it as accepted. – aaronburro May 27 '20 at 12:46

1 Answers1

0

Suppose that Foo instances are created as needed and are not permanently rooted. Is tcs.Task rooted?

No. There is no reason for tcs.Task to be routed out of Bar and request. Since you pass tcs in a lambda, it will be "closured", i.e. there will be an object created and it will be passed to both Bar and request. Once both routines end, tcs.Task will become unreachable and legible for collection.

More importantly, suppose request never completes, so the result doesn't get set. Would tcs.Task hang around forever?

If request never completes, you have more problems than just tcs.Task. It means that a whole thread will be blocked and stay alive until the end of your process, with all its stack (1 MB) and all the graph of objects that is rooted in that stack. Damage is worse if you also acquire unmanaged resources. If request fails with an exception, it will end, its stack will get released and the objects that are rooted to it will become legible for collection.

Nick
  • 4,787
  • 2
  • 18
  • 24
  • *"If request never completes, [...] It means that a whole thread will be blocked"* Why? I can't see any thread created by the OP's code. Neither I can see any thread blocked, waiting synchronously for the `tcs.Task` to complete. The `ContinueWith` continuation is accessing the `task.Result` at a point that the task is completed, so there is no blocking there. – Theodor Zoulias May 27 '20 at 11:55
  • The author says: **"// this runs asynchronously"**. Since we don't have the source of `request`, we can assume he is creating a task or a thread. – Nick May 27 '20 at 12:35
  • 1
    @Nick, no you can't assume that this creates or blocks a thread. What's to say that it doesn't store `callback`, send a network request, and correlate the response of that request to `callback`? What if `request` just chooses not to call the callback? Nothing here requires an explicit thread to be created or blocked. The comment was only there to note that `action` would relinquish control of the calling thread and call `callback` from another thread. – aaronburro May 27 '20 at 12:54
  • 1
    What's right is that the call to `request` roots `tcs`, which roots `tcs.Task`, which roots the continuation. What's wrong is the assumption that a thread is blocked. I won't upvote or downvote as a result – aaronburro May 27 '20 at 12:55
  • @aaronburro, then I suggest you show us some more code. You can't expect one to go into contemplations about all possible variations. If you want so, just use a profiler. – Nick May 27 '20 at 14:19