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

Prevent flowing ExecutionContext #1825

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
19 changes: 17 additions & 2 deletions src/Hangfire.Core/Processing/BackgroundDispatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,25 @@ public BackgroundDispatcher(

_stopped = new CountdownEvent(threads.Length);

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.
#if !NETSTANDARD1_3
using (ExecutionContext.SuppressFlow())
{
thread.Start();
#endif
foreach (var thread in threads)
{
thread.Start();
}
#if !NETSTANDARD1_3
}
#endif
}

public bool Wait(TimeSpan timeout)
Expand Down
28 changes: 22 additions & 6 deletions src/Hangfire.Core/Processing/BackgroundDispatcherAsync.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,30 @@ public BackgroundDispatcherAsync(

_stopped = new CountdownEvent(maxConcurrency);

for (var i = 0; i < maxConcurrency; i++)
// 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.
#if !NETSTANDARD1_3
using (ExecutionContext.SuppressFlow())
{
Task.Factory.StartNew(
DispatchLoop,
CancellationToken.None,
TaskCreationOptions.None,
_taskScheduler).Unwrap();
#endif
for (var i = 0; i < maxConcurrency; i++)
{
Task.Factory.StartNew(
DispatchLoop,
CancellationToken.None,
TaskCreationOptions.None,
_taskScheduler).Unwrap();
}
#if !NETSTANDARD1_3
}
#endif

}

public bool Wait(TimeSpan timeout)
Expand Down
19 changes: 17 additions & 2 deletions src/Hangfire.Core/Processing/BackgroundTaskScheduler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,25 @@ 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.
#if !NETSTANDARD1_3
using (ExecutionContext.SuppressFlow())
{
thread.Start();
#endif
foreach (var thread in _threads)
{
thread.Start();
}
#if !NETSTANDARD1_3
}
#endif

_ourThreadIds = new HashSet<int>(_threads.Select(x => x.ManagedThreadId));
}
Expand Down
28 changes: 22 additions & 6 deletions src/Hangfire.Core/Server/CoreBackgroundJobPerformer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,28 @@ private object InvokeMethod(PerformContext context, object instance, object[] ar

private object InvokeOnTaskScheduler(PerformContext context, Tuple<MethodInfo, object, object[]> tuple, Func<object, Task> getTaskFunc)
{
var scheduledTask = Task.Factory.StartNew(
InvokeSynchronously,
tuple,
CancellationToken.None,
TaskCreationOptions.None,
_taskScheduler);
// 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.
Task<object> scheduledTask;
#if !NETSTANDARD1_3
using (ExecutionContext.SuppressFlow())
{
#endif
scheduledTask = Task.Factory.StartNew(
InvokeSynchronously,
tuple,
CancellationToken.None,
TaskCreationOptions.None,
_taskScheduler);
#if !NETSTANDARD1_3
}
#endif

var result = scheduledTask.GetAwaiter().GetResult();
if (result == null) return null;
Expand Down