Skip to content

Commit

Permalink
Mono: use shared StartHelper object (#46896)
Browse files Browse the repository at this point in the history 
* Revert "Partial rollback of #46244 to workaround #46389 (#46390)"

This reverts commit 43d0780.

* [threads] Check for null longlived thread data

This might happen on netcore if the managed Thread constructor throws an
exception before calling InitInternal.  In that case the finalizer may see a
partially initialized object.
  • Loading branch information
@lambdageek
lambdageek authored Jan 13, 2021
1 parent 725f29b commit ce4c725
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 178 deletions.
2 changes: 0 additions & 2 deletions src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ private void InitializeCulture()
}
}

#if !MONO // Workaround for #46389
public Thread(ThreadStart start)
{
if (start == null)
Expand Down Expand Up @@ -237,7 +236,6 @@ private void SetCultureOnUnstartedThread(CultureInfo value, bool uiCulture)
startHelper._culture = value;
}
}
#endif // Workaround for #46389

partial void ThreadNameChanged(string? value);

Expand Down
7 changes: 5 additions & 2 deletions src/mono/mono/metadata/threads.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1940,8 +1940,11 @@ ves_icall_System_Threading_InternalThread_Thread_free_internal (MonoInternalThre
mono_threads_close_native_thread_handle (MONO_GPOINTER_TO_NATIVE_THREAD_HANDLE (this_obj->native_handle));
this_obj->native_handle = NULL;

/* Possibly free synch_cs, if the thread already detached also. */
dec_longlived_thread_data (this_obj->longlived);
/* might be null if the constructor threw an exception */
if (this_obj->longlived) {
/* Possibly free synch_cs, if the thread already detached also. */
dec_longlived_thread_data (this_obj->longlived);
}

mono_thread_name_cleanup (&this_obj->name);
}
Expand Down
189 changes: 15 additions & 174 deletions src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,7 @@ public partial class Thread
#pragma warning restore 169, 414, 649

private string? _name;
private Delegate? m_start;
private object? m_start_arg;
private CultureInfo? culture, ui_culture;
private StartHelper? _startHelper;
internal ExecutionContext? _executionContext;
internal SynchronizationContext? _synchronizationContext;

Expand Down Expand Up @@ -175,90 +173,6 @@ public ThreadPriority Priority

public ThreadState ThreadState => GetState(this);

public Thread(ThreadStart start)
: this()
{
if (start == null)
{
throw new ArgumentNullException(nameof(start));
}

Create(start);
}

public Thread(ThreadStart start, int maxStackSize)
: this()
{
if (start == null)
{
throw new ArgumentNullException(nameof(start));
}
if (maxStackSize < 0)
{
throw new ArgumentOutOfRangeException(nameof(maxStackSize), SR.ArgumentOutOfRange_NeedNonNegNum);
}

Create(start, maxStackSize);
}

public Thread(ParameterizedThreadStart start)
: this()
{
if (start == null)
{
throw new ArgumentNullException(nameof(start));
}

Create(start);
}

public Thread(ParameterizedThreadStart start, int maxStackSize)
: this()
{
if (start == null)
{
throw new ArgumentNullException(nameof(start));
}
if (maxStackSize < 0)
{
throw new ArgumentOutOfRangeException(nameof(maxStackSize), SR.ArgumentOutOfRange_NeedNonNegNum);
}

Create(start, maxStackSize);
}

private void RequireCurrentThread()
{
if (this != CurrentThread)
{
throw new InvalidOperationException(SR.Thread_Operation_RequiresCurrentThread);
}
}

private void SetCultureOnUnstartedThread(CultureInfo value, bool uiCulture)
{
if (value == null)
{
throw new ArgumentNullException(nameof(value));
}
if ((ThreadState & ThreadState.Unstarted) == 0)
{
throw new InvalidOperationException(SR.Thread_Operation_RequiresCurrentThread);
}
if (uiCulture)
ui_culture = value;
else
culture = value;
}

private void Create(ThreadStart start) => SetStartHelper((Delegate)start, 0); // 0 will setup Thread with default stackSize

private void Create(ThreadStart start, int maxStackSize) => SetStartHelper((Delegate)start, maxStackSize);

private void Create(ParameterizedThreadStart start) => SetStartHelper((Delegate)start, 0);

private void Create(ParameterizedThreadStart start, int maxStackSize) => SetStartHelper((Delegate)start, maxStackSize);

public ApartmentState GetApartmentState() => ApartmentState.Unknown;

public void DisableComObjectEagerCleanup()
Expand Down Expand Up @@ -291,10 +205,12 @@ public bool Join(int millisecondsTimeout)
return JoinInternal(this, millisecondsTimeout);
}

private void SetStartHelper(Delegate start, int maxStackSize)
private void Initialize()
{
m_start = start;
stack_size = maxStackSize;
InitInternal(this);

// TODO: This can go away once the mono/mono mirror is disabled
stack_size = _startHelper!._maxStackSize;
}

public static void SpinWait(int iterations)
Expand All @@ -316,103 +232,28 @@ public static void Sleep(int millisecondsTimeout)

internal static void UninterruptibleSleep0() => SleepInternal(0, false);

#if !TARGET_BROWSER
internal const bool IsThreadStartSupported = true;

[UnsupportedOSPlatform("browser")]
public void Start()
{
_executionContext = ExecutionContext.Capture();
StartInternal(this);
}

[UnsupportedOSPlatform("browser")]
public void Start(object parameter)
{
if (m_start is ThreadStart)
throw new InvalidOperationException(SR.InvalidOperation_ThreadWrongThreadStart);

m_start_arg = parameter;
Start();
}

[UnsupportedOSPlatform("browser")]
internal void UnsafeStart()
{
StartInternal(this);
}

[UnsupportedOSPlatform("browser")]
internal void UnsafeStart(object parameter)
{
Debug.Assert(m_start is ThreadStart);

m_start_arg = parameter;
UnsafeStart();
}

// Called from the runtime
internal void StartCallback()
{
ExecutionContext? context = _executionContext;
_executionContext = null;
if (context != null && !context.IsDefault)
{
ExecutionContext.RunInternal(context, s_threadStartContextCallback, this);
}
else
{
StartCallbackWorker();
}
}

private static readonly ContextCallback s_threadStartContextCallback = new ContextCallback(StartCallback_Context);

private static void StartCallback_Context(object? state)
{
Debug.Assert(state is Thread);
((Thread)state).StartCallbackWorker();
}

[MethodImpl(MethodImplOptions.AggressiveInlining)] // otherwise an unnecessary long-lived stack frame in many threads
private void StartCallbackWorker()
{
if (culture != null)
{
CultureInfo.CurrentCulture = culture;
culture = null;
}
StartHelper? startHelper = _startHelper;
Debug.Assert(startHelper != null);
_startHelper = null;

if (ui_culture != null)
{
CultureInfo.CurrentUICulture = ui_culture;
ui_culture = null;
}

if (m_start is ThreadStart del)
{
m_start = null;
del();
}
else
{
Debug.Assert(m_start is ParameterizedThreadStart);
var pdel = (ParameterizedThreadStart)m_start!;
object? arg = m_start_arg;
m_start = null;
m_start_arg = null;
pdel(arg);
}
startHelper.Run();
}

// Called from the runtime
internal static void ThrowThreadStartException(Exception ex) => throw new ThreadStartException(ex);

private void StartCore()
{
StartInternal(this);
}

[DynamicDependency(nameof(StartCallback))]
[DynamicDependency(nameof(ThrowThreadStartException))]
[MethodImplAttribute(MethodImplOptions.InternalCall)]
private static extern void StartInternal(Thread runtime_thread);
#endif

partial void ThreadNameChanged(string? value)
{
Expand Down

0 comments on commit ce4c725

Please sign in to comment.