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

[mono] Fix deadlock in static constructor initialization #93875

Merged
merged 6 commits into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from 5 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
51 changes: 49 additions & 2 deletions src/mono/mono/metadata/object.c
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ mono_runtime_class_init_full (MonoVTable *vtable, MonoError *error)
* on this cond var.
*/

retry_top:
mono_type_initialization_lock ();
/* double check... */
if (vtable->initialized) {
Expand Down Expand Up @@ -506,6 +507,12 @@ mono_runtime_class_init_full (MonoVTable *vtable, MonoError *error)
blocked = GUINT_TO_POINTER (MONO_NATIVE_THREAD_ID_TO_UINT (lock->initializing_tid));
while ((pending_lock = (TypeInitializationLock*) g_hash_table_lookup (blocked_thread_hash, blocked))) {
if (mono_native_thread_id_equals (pending_lock->initializing_tid, tid)) {
if (mono_trace_is_traced (G_LOG_LEVEL_DEBUG, MONO_TRACE_TYPE)) {
char* type_name = mono_type_full_name (m_class_get_byval_arg (klass));
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_TYPE, "Detected deadlock for class .cctor for %s from '%s'", type_name, m_class_get_image (klass)->name);
g_free (type_name);
}

if (!pending_lock->done) {
mono_type_initialization_unlock ();
goto return_true;
Expand Down Expand Up @@ -604,9 +611,49 @@ mono_runtime_class_init_full (MonoVTable *vtable, MonoError *error)
} else {
/* this just blocks until the initializing thread is done */
mono_type_init_lock (lock);
while (!lock->done)
mono_coop_cond_wait (&lock->cond, &lock->mutex);
if (!lock->done) {
int timeout_ms = 500;
int wait_result = mono_coop_cond_timedwait (&lock->cond, &lock->mutex, timeout_ms);
if (wait_result == -1) {
/* timed out - go around again from the beginning. If we got here
* from the "is_blocked = FALSE" case, above (another thread was
* blocked on the current thread, but on a lock that was already
* done but it didn't get to wake up yet), then it might still be
* the case that the current thread cannot proceed even if the other
* thread got to wake up - there might be a new deadlock. We need
* to re-evaluate.
*
* This can happen if two threads A and B need to call the cctors
* for classes X and Y but in opposite orders, and also call a cctor
* for a third class Z. (That is thread A wants to init: X, Z, Y;
* thread B wants to init: Y, Z, X.) In that case, if B is waiting
* for A to finish initializing Z, and A (the current thread )
* already finished Z and wants to init Y. In A, control will come
* here with "lock" being Y's lock. But we will time out because B
* will see that A is responsible for initializing X and will also
* block. So A is waiting for B to finish Y and B is waiting for A
* to finish X. So the fact that A allowed B to wait for Z to
* finish didn't actually let us make progress. Thread A must go
* around to the top once more and try to init Y - and detect that
* there is now a deadlock between X and Y.
*/
mono_type_init_unlock (lock);
// clean up blocked thread hash and lock refcount.
mono_type_initialization_lock ();
g_hash_table_remove (blocked_thread_hash, GUINT_TO_POINTER (tid));
gboolean deleted = unref_type_lock (lock);
if (deleted)
g_hash_table_remove (type_initialization_hash, vtable);
mono_type_initialization_unlock ();
goto retry_top;
} else if (wait_result == 0) {
/* success.. we were signaled that the other thread is done. Proceed */
} else {
g_error ("unexpected return value %d from mono_coop_cond_timedwait", wait_result);
lambdageek marked this conversation as resolved.
Show resolved Hide resolved
}
}
mono_type_init_unlock (lock);
g_assert (lock->done);
}

/* Do cleanup and setting vtable->initialized inside the global lock again */
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Runtime.CompilerServices;
using System.Threading;

using Xunit;

// Regression test for https://github.com/dotnet/runtime/issues/93778
namespace CircularCctorTwoThreadsThreeCC;

[Flags]
public enum SlotConstants : int
{
ZInited = 1,
YInitedFromX = 2,
XInitedFromY = 4,

Thread1 = 1 << 16,
Thread2 = 2 << 16,
}

/// X and Y both try to use each other, and also both use Z.
/// We expect to see exactly one thread initialize Z and
/// either X inited from Y or Y inited from X.
public class X
{
public static X Singleton = new();
private X() {
Z.Singleton.Ping();
Y.Singleton?.Pong();
}

public void Pong() => Coordinator.Note(SlotConstants.XInitedFromY);
}

public class Y
{
public static Y Singleton = new();
private Y() {
Z.Singleton.Ping();
X.Singleton?.Pong();
}

public void Pong() => Coordinator.Note(SlotConstants.YInitedFromX);
}

public class Z
{
public static Z Singleton = new();

private Z() {
Coordinator.Note(SlotConstants.ZInited);
}

public void Ping() { }

}

public class Coordinator
{
[ThreadStatic]
private static SlotConstants t_threadTag;

private static int s_NextNote;
private static readonly SlotConstants[] Notes = new SlotConstants[12];

private static SlotConstants DecorateWithThread(SlotConstants c)
{
return c | t_threadTag;
}

public static void Note(SlotConstants s) {
int idx = Interlocked.Increment(ref s_NextNote);
idx--;
Notes[idx] = DecorateWithThread (s);
}

public static Coordinator CreateThread(bool xThenY, SlotConstants threadTag)
{
return new Coordinator(xThenY, threadTag);
}

public readonly Thread Thread;
private static readonly Barrier s_barrier = new (3);

private Coordinator(bool xThenY, SlotConstants threadTag)
{
var t = new Thread(() => {
t_threadTag = threadTag;
// Log("started");
NextPhase();
// Log("racing");
DoConstructions(xThenY);
NextPhase();
// Log("done");
});
Thread = t;
t.Start();
}

public static void NextPhase() { s_barrier.SignalAndWait(); }

[MethodImpl(MethodImplOptions.NoInlining)]
public static void DoConstructions(bool xThenY)
{
if (xThenY) {
XCreate();
} else {
YCreate();
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static void XCreate()
{
var _ = X.Singleton;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static void YCreate()
{
var _ = Y.Singleton;
}

public static void Log(string msg)
{
Console.WriteLine ($"{Thread.CurrentThread.ManagedThreadId}: {msg}");
}

[Fact]
public static void RunTestCase()
{
var c1 = CreateThread(xThenY: true, threadTag: SlotConstants.Thread1);
var c2 = CreateThread(xThenY: false, threadTag: SlotConstants.Thread2);
// created all threads
NextPhase();
// racing
NextPhase();
// done

// one second should be plenty for all these threads, but it's arbitrary
int threadJoinTimeoutMS = 1000;
var j1 = c1.Thread.Join(threadJoinTimeoutMS);
var j2 = c2.Thread.Join(threadJoinTimeoutMS);
Assert.True(j1);
Assert.True(j2);
// all joined

// exactly one thread inited Z
Assert.Equal(1, CountNotes(SlotConstants.ZInited));
// either X was inited or Y, not both.
Assert.Equal(1, Count2Notes(SlotConstants.XInitedFromY, SlotConstants.YInitedFromX));
}

private static int CountNotes(SlotConstants mask)
{
int found = 0;
foreach (var note in Notes) {
if ((note & mask) != (SlotConstants)0) {
found++;
}
}
return found;
}

private static int Count2Notes(SlotConstants mask1, SlotConstants mask2)
{
int found = 0;
foreach (var note in Notes) {
if ((note & mask1) != (SlotConstants)0) {
found++;
}
if ((note & mask2) != (SlotConstants)0) {
found++;
}
}
return found;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<CLRTestPriority>0</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="CircularCctorTwoThreadsThreeCC.cs" />
</ItemGroup>
</Project>
3 changes: 3 additions & 0 deletions src/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -3221,6 +3221,9 @@
<ExcludeList Include = "$(XunitTestBinBase)/tracing/eventcounter/pollingcounter/**">
<Issue>System.Threading.Thread.UnsafeStart not supported</Issue>
</ExcludeList>
<ExcludeList Include = "$(XunitTestBinBase)/Loader/classloader/TypeInitialization/CircularCctors/CircularCctorTwoThreadsThreeCC/**">
<Issue>System.Threading.Thread.ThrowIfNoThreadStart: PlatformNotSupportedException</Issue>
</ExcludeList>
<ExcludeList Include = "$(XunitTestBinBase)/tracing/eventcounter/gh53564/**">
<Issue>Could not load legacy Microsoft.Diagnostics.Tools.RuntimeClient</Issue>
</ExcludeList>
Expand Down
Loading