Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JoinableTask can be leaked easily through ExecutionContext, and causes task results to be leaked #274

Closed
lifengl opened this issue May 11, 2018 · 3 comments
Assignees
Labels
Milestone

Comments

@lifengl
Copy link
Member

lifengl commented May 11, 2018

ExecutionContext is copied very often in any place to create a task continuation, or spin off an execution. The context will capture the current ambient task, and hold it even the task has been completed for a long time. This can easily leak the task result, which can be very big.

@lifengl lifengl added the bug label May 11, 2018
@AArnott
Copy link
Member

AArnott commented May 11, 2018

@lifengl adds:

It is only when JTF.RunAsync/JTF.Run is used. When T is a simple value, it is generally not a problem. But when T is a complex value, it can be a problem.

For example

JTF.RunAsync<T> (async () =>
// …
someBackGroupThread.Start();
// …
});

If the background thread is long running thread, the returning value of the JTF task will be leaked. Of course, generally, we don’t start a such task inside a JTF task. Often, it happens when we initialize some other components. For example, a build manager may spin off a background task, so when we accidently initialize it within a JTF, it can cause the leak.

Another common place is to chain a dataflow link. When we ask it to PropagateCompletion, it will chain a task continuation, which captures whatever the current JTF task.

Of course, in many places, we can if it with JTFContext.SuppressRelevance(). But in the reality, it can happen in many places, which make it hard to apply.

@lifengl
Copy link
Member Author

lifengl commented Jun 5, 2018

It is more than spinning off a task. More often, it is the lazy initialization code to chain up data source completion, or long term cancellation token registration (like project unloading cancellation token). All those will capture the current ExecutionContext, and apply them much later (often at the end of the session). That leads it to capture whatever the current JTF task, which happens to trigger the lazy calculation, and its result.

@AArnott
Copy link
Member

AArnott commented Aug 12, 2018

I wonder if we can fix this by using a AsyncLocal<WeakReference<JoinableTask>>, thereby ensuring we never keep them around longer than is needful. One problem I see with this is we'd allocate another object, but maybe on the scale of allocations JoinableTask already make, that's not a big deal. Also, each JoinableTask can maintain its own weak reference in a field so we can reuse it rather than create a new one each time we assign to the AsyncLocal.

@AArnott AArnott self-assigned this Aug 12, 2018
@AArnott AArnott added this to the v16.0 milestone Aug 12, 2018
AArnott added a commit to AArnott/vs-threading that referenced this issue Aug 12, 2018
AArnott added a commit to AArnott/vs-threading that referenced this issue Aug 12, 2018
Fixes microsoft#274 and gets the recently added test to pass
AArnott added a commit to AArnott/vs-threading that referenced this issue Aug 12, 2018
Fixes microsoft#274 and gets the recently added test to pass
AArnott added a commit to AArnott/vs-threading that referenced this issue Aug 13, 2018
AArnott added a commit to AArnott/vs-threading that referenced this issue Aug 13, 2018
Fixes microsoft#274 and gets the recently added test to pass
AArnott added a commit to AArnott/vs-threading that referenced this issue Jun 18, 2024
Ensure Expand-Template uses UTF-8 when replacing placeholders.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants