Skip to content

Prevent flowing ExecutionContext #1825

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Prevent flowing ExecutionContext
Prevent flowing the Execution Context to background threads. Without this, it means values from AsyncLocal and CallContext will flow/leak into background threads which should be avoided because it may inadvertently be capturing some ambient contexts like DbContext, etc... which can have unwanted side affects. In most cases anything stored in AsyncLocal is also not thread safe and if it's a mutable object it means concurrent threads can be modifying it which also may cause unwanted side affects. I don't think there's a use-case for flowing the execution context into a background thread since the execution in these threads should be entirely isolated.

I put together a gist showing all the different ways the AsyncLocal/CallContext values flow to child threads here https://gist.github.com/Shazwazza/c127e8c567ab505d146e54ac802a9945

I think this is the only place where the created threads are actually started but I could be wrong. Calling SuppressFlow is only required where the thread is started.
  • Loading branch information
Shazwazza authored Mar 9, 2021
commit 5aed38b46ea018bcf4aed09bc89b6736ea6cf94c
17 changes: 14 additions & 3 deletions src/Hangfire.Core/Processing/BackgroundTaskScheduler.cs
Original file line number Diff line number Diff line change
@@ -131,10 +131,21 @@ public BackgroundTaskScheduler(
throw new ArgumentException("All the threads should be non-null and in the ThreadState.Unstarted state.", nameof(threadFactory));
}

foreach (var thread in _threads)
// Prevent flowing the Execution Context to background threads.
// Without this, it means values from AsyncLocal and CallContext
// will flow/leak into background threads which should be avoided
// because it may inadvertently be capturing some ambient contexts
// like DbContext, etc... which can have unwanted side affects.
// In most cases anything stored in AsyncLocal is also not thread
// safe and if it's a mutable object it means concurrent threads can
// be modifying it which also may cause unwanted side affects.
using (ExecutionContext.SuppressFlow())
{
thread.Start();
}
foreach (var thread in _threads)
{
thread.Start();
}
}

_ourThreadIds = new HashSet<int>(_threads.Select(x => x.ManagedThreadId));
}