Simple logic (requirement) but quite complex solution

[stevenxi]

Memory leak caused by AsyncLocal<> from PerLogicalCallContextScopeManager

Hi,
Many thanks for this project. We've used it for web for years, it works fine.
But recently we've adopted the implementation into a Windows service. Got bit of problem.

Bit explanation on background:
Our service is driven/triggered by a message bus. When message comes in, it creates a new Task. And this task runs a long-running job.
Also, there're timers to trigger scheduled jobs.

Each job was suppose to be wrapped within it's own scope.

Our need sounds very simple. There's a root scope, it creates all children scopes. Each child scope used for one job. Jobs may run parallel. The job is ran through Task.

The problem is the ScopeManager. Currently there're 2 types of ScopeManager (PerThreadScopeManager & PerLogicalCallContextScopeManager), but neither fit our needs.

Obviously the PerThreadScopeManager not work in this case.
So we tried PerLogicalCallContextScopeManager. But due to performance consideration, we uses .ConfigureAwait(false) everywhere (in the backend service). It causes quite bad memory leak, as the context switches, the ExecutionContext shadow-clones its' Dictionary<IAsyncLocal, object>. Ends up there're many Sope been disposed but not cleaned by GC, due to reference to the Dictionary<IAsyncLocal, object>s.

So I've added a new implementation of ScopeManager #449 , that can pass on the parent/scope relationship as non-static way. It seems there's no way to approach it with 'clean' and 'nice' code. Trouble is that current Scope and ServiceContainer design assumes the ScopeManger.CurrentScope always used by the thread. So my solution looks bit complex.

I'm not sure if there's better way to do this?

[stevenxi]

Just bit update.
It has nothing to do with ConfigureAwait(false)

According to https://referencesource.microsoft.com/#mscorlib/system/threading/executioncontext.cs,dcc44d583d37c571

when set value to the ExecutionContext (which handles the AsyncLocal's value), it always creates a copy of _localValues. And that's where holds the Scope that suppose to be cleaned up.

[seesharper]

Hi and thanks for reporting this.

Looks to me that this is a bug related to AsyncLocal<T> and/or ExecutionContext

Tried to search for any evidence for memory issues

https://twitter.com/davidfowl/status/1033964051607379968

dotnet/aspnetcore#3249

Is it possible to come up with a simple repro of this? If not as a test, just a simple console app of some kind that we can use to monitor memory. The best solution here would be to fix the PerLogicalCallContextScopeManager rather than introducing a new scope manager. At least I would prefer looking into that first.

Maybe we can use a WeakReference to the scope or something. I don't know.

If you could help me with a small repro of this, that would be super useful 💪

And we take it from there.

[stevenxi]

@seesharper It may not necessary to be a bug related to AsyncLocal
My local profiling only waited few minutes. "Maybe" the underlying ExecutionContext will be cleaned up but takes longer. It may not (I'm not sure) design to hold very large objects.

Apart from the pull request I've created #449. Another suggestion would be, clean the HashSet inside Scope after dispose them. So they won't be referenced.

I've tried to create a simple solution out of app. Not finished yet. Will provide once done.

[seesharper]

Appreciate it 👍

[seesharper]

@stevenxi Did you manage to create a repro of this ? 😎

[stevenxi]

@seesharper Unfortunately not. Unless I have full set of message bus + mongodb in place...