-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix leaky coordinated shutdown #5816
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -251,7 +251,7 @@ private ClusterJoinUnsuccessfulReason() { } | |
/// <summary> | ||
/// The <see cref="ActorSystem"/> | ||
/// </summary> | ||
public ExtendedActorSystem System { get; } | ||
public ExtendedActorSystem System { get; private set; } | ||
|
||
/// <summary> | ||
/// The set of named <see cref="Phase"/>s that will be executed during coordinated shutdown. | ||
|
@@ -261,7 +261,7 @@ private ClusterJoinUnsuccessfulReason() { } | |
/// <summary> | ||
/// INTERNAL API | ||
/// </summary> | ||
internal ILoggingAdapter Log { get; } | ||
internal ILoggingAdapter Log { get; private set; } | ||
|
||
private readonly HashSet<string> _knownPhases; | ||
|
||
|
@@ -270,7 +270,7 @@ private ClusterJoinUnsuccessfulReason() { } | |
/// </summary> | ||
internal readonly List<string> OrderedPhases; | ||
|
||
private readonly ConcurrentBag<Func<Task<Done>>> _clrShutdownTasks = new ConcurrentBag<Func<Task<Done>>>(); | ||
private readonly ConcurrentSet<Func<Task<Done>>> _clrShutdownTasks = new ConcurrentSet<Func<Task<Done>>>(); | ||
private readonly ConcurrentDictionary<string, ImmutableList<(string, Func<Task<Done>>)>> _tasks = new ConcurrentDictionary<string, ImmutableList<(string, Func<Task<Done>>)>>(); | ||
private readonly AtomicReference<Reason> _runStarted = new AtomicReference<Reason>(null); | ||
private readonly AtomicBoolean _clrHooksStarted = new AtomicBoolean(false); | ||
|
@@ -335,7 +335,7 @@ internal void AddClrShutdownHook(Func<Task<Done>> hook) | |
{ | ||
if (!_clrHooksStarted) | ||
{ | ||
_clrShutdownTasks.Add(hook); | ||
_clrShutdownTasks.TryAdd(hook); | ||
} | ||
} | ||
|
||
|
@@ -653,13 +653,16 @@ internal static void InitPhaseActorSystemTerminate(ActorSystem system, Config co | |
|
||
if (terminateActorSystem) | ||
{ | ||
system.FinalTerminate(); | ||
return system.Terminate().ContinueWith(tr => | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This potentially an infinite loop call, if the call came from |
||
return system.FinalTerminate().ContinueWith(tr => | ||
{ | ||
if (exitClr && !coord._runningClrHook) | ||
{ | ||
Environment.Exit(0); | ||
} | ||
|
||
coord.System = null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clear the reference to the |
||
coord.Log = null; | ||
coord._tasks.Clear(); // Clear the dictionary, just in case it is retained in memory | ||
return Done.Instance; | ||
}); | ||
} | ||
|
@@ -691,7 +694,11 @@ internal static void InitClrHook(ActorSystem system, Config conf, CoordinatedShu | |
var exitTask = TerminateOnClrExit(coord); | ||
// run all hooks during termination sequence | ||
AppDomain.CurrentDomain.ProcessExit += exitTask; | ||
system.WhenTerminated.ContinueWith(tr => { AppDomain.CurrentDomain.ProcessExit -= exitTask; }); | ||
system.WhenTerminated.ContinueWith(tr => | ||
{ | ||
AppDomain.CurrentDomain.ProcessExit -= exitTask; | ||
coord._clrShutdownTasks.Clear(); // Clear the tasks, just in case it is retained in memory | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm pretty sure that |
||
}); | ||
|
||
coord.AddClrShutdownHook(() => | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the real cause of the memory leak.
ConcurrentBag
usesThreadLocal
that pinned the lambdas in memory, preventing them from being GC-ed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yikes