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.

Sign up for GitHub

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

Jump to bottom

Console.Unix: avoid deadlock between LazyInitializer and Console.Out #34297

Merged
merged 32 commits into from
Apr 13, 2020
Merged

Console.Unix: avoid deadlock between LazyInitializer and Console.Out #34297

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
d0f5dc1
Console.Unix: avoid deadlock between LazyInitializer and Console.Out
tmds Mar 30, 2020
fae6ccb
Add assert for InternalSyncObject
tmds Mar 30, 2020
1ff6d97
Console: use Interlocked for lazy initialization
tmds Mar 31, 2020
69473a8
Cleanup unused 'using'
tmds Mar 31, 2020
d2b4e91
Limit locking to event handlers
tmds Mar 31, 2020
c4c09c2
Fix AllowNull -> NotNull
tmds Mar 31, 2020
7de5274
Localize LazyInitializer.EnsureInitialized logic
tmds Mar 31, 2020
af57dde
Remove lazy initialize delegates
tmds Mar 31, 2020
d5bf05f
Use static local functions for initializing
tmds Apr 1, 2020
2043770
Remove unused EnsureInitializedDisposableCore
tmds Apr 1, 2020
008d833
Remove unused arg from EnsureInitializedStdInReader
tmds Apr 1, 2020
8e0b37f
Also EnsureInitialize when not redirected
tmds Apr 1, 2020
dc533a7
Dispose Console Streams with their Reader/Writer
tmds Apr 2, 2020
3080288
Fix s_out -> s_error
tmds Apr 2, 2020
77c2c2f
leaveOpen streams, fix Encoding set race
tmds Apr 2, 2020
0f48a86
Fix races between Encoding en initializers of In/Out/Error
tmds Apr 2, 2020
f4656f0
Remove EnsureInitialized
tmds Apr 2, 2020
73d24c4
Cleanup
tmds Apr 2, 2020
5b1f471
Rename static local initializer functions to EnsureInitialized
tmds Apr 3, 2020
3500753
Use lock in Input/OutputEncoding
tmds Apr 3, 2020
614f621
Rename InternalSyncObject to s_syncObject
tmds Apr 3, 2020
1fe70bd
Cleanup space
tmds Apr 6, 2020
10ae846
Fix _isStdErrRedirected -> _isStdInRedirected
tmds Apr 6, 2020
f07e5ce
Remove space
tmds Apr 6, 2020
dcf7116
Improve comments
tmds Apr 6, 2020
b61ee9c
Fix potential CancelKeyPress deadlock
tmds Apr 7, 2020
705b104
Apply suggestions from code review
tmds Apr 7, 2020
33d5b5c
Remove unnecessary null forgiving
tmds Apr 7, 2020
61349ef
Add comment about why we're not Disposing StdInReader
tmds Apr 7, 2020
f19a34b
Add Volatile.Writes for Volatile.Read fields
tmds Apr 8, 2020
f009a74
More Volatile.Writes
tmds Apr 8, 2020
a633fdc
Add back lock to SetIn
tmds Apr 8, 2020
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
34 changes: 23 additions & 11 deletions src/libraries/System.Console/src/System/Console.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public static class Console
// there's little benefit to having a large buffer. So we use a smaller buffer size to reduce working set.
private const int WriteBufferSize = 256;

private static object InternalSyncObject = new object(); // for synchronizing changing of Console's static fields
internal static object InternalSyncObject = new object(); // for synchronizing changing of Console's static fields
tmds marked this conversation as resolved.
Show resolved Hide resolved
private static TextReader? s_in;
private static TextWriter? s_out, s_error;
private static Encoding? s_inputEncoding;
Expand All @@ -33,16 +33,28 @@ public static class Console
private static ConsoleCancelEventHandler? s_cancelCallbacks;
private static ConsolePal.ControlCHandlerRegistrar? s_registrar;

internal static T EnsureInitialized<T>([NotNull] ref T? field, Func<T> initializer) where T : class =>
LazyInitializer.EnsureInitialized(ref field, ref InternalSyncObject, initializer);
internal static T EnsureInitializedDisposable<T>([AllowNull] ref T? field, Func<T> initializer) where T : class, IDisposable
tmds marked this conversation as resolved.
Show resolved Hide resolved
=> Volatile.Read(ref field) ?? EnsureInitializedDisposableCore(ref field, initializer);

public static TextReader In => EnsureInitialized(ref s_in, () => ConsolePal.GetOrCreateReader());
private static T EnsureInitializedDisposableCore<T>([AllowNull] ref T? field, Func<T> initializer) where T : class, IDisposable
tmds marked this conversation as resolved.
Show resolved Hide resolved
{
T value = initializer();

if (Interlocked.CompareExchange(ref field, value, null) != null)
{
value.Dispose();
}

return field;
}

public static TextReader In => EnsureInitializedDisposable(ref s_in, () => ConsolePal.GetOrCreateReader());

public static Encoding InputEncoding
{
get
{
return EnsureInitialized(ref s_inputEncoding, () => ConsolePal.InputEncoding);
return LazyInitializer.EnsureInitialized(ref s_inputEncoding, () => ConsolePal.InputEncoding);
tmds marked this conversation as resolved.
Show resolved Hide resolved
tmds marked this conversation as resolved.
Show resolved Hide resolved
}
set
{
Expand All @@ -66,7 +78,7 @@ public static Encoding OutputEncoding
{
get
{
return EnsureInitialized(ref s_outputEncoding, () => ConsolePal.OutputEncoding);
return LazyInitializer.EnsureInitialized(ref s_outputEncoding, () => ConsolePal.OutputEncoding);
}
set
{
Expand Down Expand Up @@ -119,9 +131,9 @@ public static ConsoleKeyInfo ReadKey(bool intercept)
return ConsolePal.ReadKey(intercept);
}

public static TextWriter Out => EnsureInitialized(ref s_out, () => CreateOutputWriter(OpenStandardOutput()));
public static TextWriter Out => EnsureInitializedDisposable(ref s_out, () => CreateOutputWriter(OpenStandardOutput()));

public static TextWriter Error => EnsureInitialized(ref s_error, () => CreateOutputWriter(OpenStandardError()));
public static TextWriter Error => EnsureInitializedDisposable(ref s_error, () => CreateOutputWriter(OpenStandardError()));

private static TextWriter CreateOutputWriter(Stream outputStream)
{
Expand All @@ -145,7 +157,7 @@ public static bool IsInputRedirected
{
get
{
StrongBox<bool> redirected = EnsureInitialized(ref _isStdInRedirected, () => new StrongBox<bool>(ConsolePal.IsInputRedirectedCore()));
StrongBox<bool> redirected = LazyInitializer.EnsureInitialized(ref _isStdInRedirected, () => new StrongBox<bool>(ConsolePal.IsInputRedirectedCore()));
return redirected.Value;
}
}
Expand All @@ -154,7 +166,7 @@ public static bool IsOutputRedirected
{
get
{
StrongBox<bool> redirected = EnsureInitialized(ref _isStdOutRedirected, () => new StrongBox<bool>(ConsolePal.IsOutputRedirectedCore()));
StrongBox<bool> redirected = LazyInitializer.EnsureInitialized(ref _isStdOutRedirected, () => new StrongBox<bool>(ConsolePal.IsOutputRedirectedCore()));
return redirected.Value;
}
}
Expand All @@ -163,7 +175,7 @@ public static bool IsErrorRedirected
{
get
{
StrongBox<bool> redirected = EnsureInitialized(ref _isStdErrRedirected, () => new StrongBox<bool>(ConsolePal.IsErrorRedirectedCore()));
StrongBox<bool> redirected = LazyInitializer.EnsureInitialized(ref _isStdErrRedirected, () => new StrongBox<bool>(ConsolePal.IsErrorRedirectedCore()));
return redirected.Value;
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/libraries/System.Console/src/System/ConsolePal.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ private static SyncTextReader StdInReader
{
EnsureInitialized();

return Console.EnsureInitialized(
return Console.EnsureInitializedDisposable(
ref s_stdInReader,
() => SyncTextReader.GetSynchronizedTextReader(
new StdInReader(
Expand Down Expand Up @@ -928,6 +928,8 @@ private static void EnsureInitialized()
/// <summary>Ensures that the console has been initialized for use.</summary>
private static void EnsureInitializedCore()
{
Debug.Assert(!Monitor.IsEntered(Console.InternalSyncObject)); // see LazyInitializeConsoleAndFieldCore
tmds marked this conversation as resolved.
Show resolved Hide resolved

stephentoub marked this conversation as resolved.
Show resolved Hide resolved
lock (Console.Out) // ensure that writing the ANSI string and setting initialized to true are done atomically
{
if (!s_initialized)
Expand Down