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

Fix handling of collectible thread statics and GC #40671

Merged
merged 2 commits into from
Aug 11, 2020
Merged
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
2 changes: 1 addition & 1 deletion src/coreclr/src/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7401,6 +7401,7 @@ GenTreeCall* Compiler::fgGetStaticsCCtorHelper(CORINFO_CLASS_HANDLE cls, CorInfo
case CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE_DYNAMICCLASS:
case CORINFO_HELP_GETSHARED_GCTHREADSTATIC_BASE:
case CORINFO_HELP_GETSHARED_GCTHREADSTATIC_BASE_DYNAMICCLASS:
case CORINFO_HELP_GETSHARED_NONGCTHREADSTATIC_BASE_DYNAMICCLASS:
// type = TYP_BYREF;
break;

Expand All @@ -7414,7 +7415,6 @@ GenTreeCall* Compiler::fgGetStaticsCCtorHelper(CORINFO_CLASS_HANDLE cls, CorInfo

case CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE:
case CORINFO_HELP_GETSHARED_NONGCTHREADSTATIC_BASE:
case CORINFO_HELP_GETSHARED_NONGCTHREADSTATIC_BASE_DYNAMICCLASS:
case CORINFO_HELP_CLASSINIT_SHARED_DYNAMICCLASS:
type = TYP_I_IMPL;
break;
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,9 @@
<ExcludeList Include="$(XunitTestBinBase)/Loader/CollectibleAssemblies/ByRefLocals/**">
<Issue>https://github.com/dotnet/runtime/issues/40394</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/Loader/CollectibleAssemblies/Statics/**">
<Issue>https://github.com/dotnet/runtime/issues/40394</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/Interop/PInvoke/Miscellaneous/CopyCtor/**">
<Issue>Handling for Copy constructors isn't present in mono interop</Issue>
</ExcludeList>
Expand Down
29 changes: 0 additions & 29 deletions src/tests/Loader/CollectibleAssemblies/ByRefLocals/ByRefLocals.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,6 @@ static int Main(string[] args)
[MethodImpl(MethodImplOptions.NoInlining)]
static int HoldAssembliesAliveThroughByRefFields(out GCHandle gch1, out GCHandle gch2)
{
// ThreadStatic lifetime check. Here we don't require the actual assembly to remain loaded, but we do require the field to remain accessible
var spanThreadStatic = LoadAssemblyThreadStatic(out GCHandle gchThreadStatic);
GC.Collect(2);
GC.WaitForPendingFinalizers();
GC.Collect(2);
GC.WaitForPendingFinalizers();
GC.Collect(2);
GC.WaitForPendingFinalizers();

var span1 = LoadAssembly(out gch1);
GC.Collect(2);
GC.WaitForPendingFinalizers();
Expand All @@ -61,7 +52,6 @@ static int HoldAssembliesAliveThroughByRefFields(out GCHandle gch1, out GCHandle
{
Console.WriteLine(span1[0]);
Console.WriteLine(span2[0]);
Console.WriteLine(spanThreadStatic[0]);
GC.Collect(2);
GC.WaitForPendingFinalizers();
if (gch1.Target == null)
Expand All @@ -72,11 +62,6 @@ static int HoldAssembliesAliveThroughByRefFields(out GCHandle gch1, out GCHandle
{
return 2;
}
if (spanThreadStatic[0] != 7)
{
Console.WriteLine($"spanThreadStatic[0] = {spanThreadStatic[0]}");
return 5;
}
}

return 100;
Expand All @@ -96,20 +81,6 @@ private static ReadOnlySpan<byte> LoadAssembly(out GCHandle gchToAssembly)
return spanAccessor.GetSpan();
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static ReadOnlySpan<byte> LoadAssemblyThreadStatic(out GCHandle gchToAssembly)
{
var alc = new AssemblyLoadContext("test", isCollectible: true);
var a = alc.LoadFromAssemblyPath(Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "Unloaded.dll"));
gchToAssembly = GCHandle.Alloc(a, GCHandleType.WeakTrackResurrection);

var spanAccessor = (IReturnSpan)Activator.CreateInstance(a.GetType("ThreadStaticSpanAccessor"));

alc.Unload();

return spanAccessor.GetSpan();
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static ReadOnlySpan<byte> CreateAssemblyDynamically(out GCHandle gchToAssembly)
{
Expand Down
11 changes: 0 additions & 11 deletions src/tests/Loader/CollectibleAssemblies/ByRefLocals/Unloaded.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,3 @@ public ReadOnlySpan<byte> GetSpan()
return RawData;
}
}

public class ThreadStaticSpanAccessor : IReturnSpan
{
[ThreadStatic]
public static byte ThreadStaticByte = 7;

public unsafe ReadOnlySpan<byte> GetSpan()
{
return new ReadOnlySpan<byte>(Unsafe.AsPointer(ref ThreadStaticByte), 1);
}
}
12 changes: 12 additions & 0 deletions src/tests/Loader/CollectibleAssemblies/Statics/Accessor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;

public interface IStaticTest
{
void SetStatic(int val, int val2, int val3, int val4, int val5);
void GetStatic(out int val1, out int val2, out int val3, out int val4, out int val5);
void SetStaticObject(object val, object val2, object val3, object val4, object val5);
void GetStaticObject(out object val1, out object val2, out object val3, out object val4, out object val5);
}
11 changes: 11 additions & 0 deletions src/tests/Loader/CollectibleAssemblies/Statics/Accessor.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<OutputType>Library</OutputType>
<CLRTestKind>BuildOnly</CLRTestKind>
<CLRTestPriority>0</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="Accessor.cs" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// 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.IO;
using System.Reflection;
using System.Reflection.Emit;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Runtime.Loader;

class Program
{
static int Main(string[] args)
{
var alc = new AssemblyLoadContext("test", isCollectible: true);
var a = alc.LoadFromAssemblyPath(Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "Unloaded.dll"));

var accessor = (IStaticTest)Activator.CreateInstance(a.GetType("StaticTest"));
accessor.SetStatic(12759, 548739, 5468, 8518, 9995);
accessor.GetStatic(out int val1, out int val2, out int val3, out int val4, out int val5);
if (val1 != 12759)
return 1;
if (val2 != 548739)
return 2;
if (val3 != 5468)
return 3;
if (val4 != 8518)
return 4;
if (val5 != 9995)
return 5;

object obj1 = new object();
object obj2 = new object();
object obj3 = new object();
object obj4 = new object();
object obj5 = new object();
accessor.SetStaticObject(obj1, obj2, obj3, obj4, obj5);
accessor.GetStaticObject(out object val1Obj, out object val2Obj, out object val3Obj, out object val4Obj, out object val5Obj);
if (val1Obj != obj1)
return 11;
if (val2Obj != obj2)
return 12;
if (val3Obj != obj3)
return 13;
if (val4Obj != obj4)
return 14;
if (val5Obj != obj5)
return 15;

GC.KeepAlive(accessor);
return 100;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<OutputType>Exe</OutputType>
<CLRTestKind>BuildAndRun</CLRTestKind>
<CLRTestPriority>0</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<ProjectReference Include="Unloaded.csproj" />
<ProjectReference Include="Accessor.csproj" />
<Compile Include="CollectibleStatics.cs" />
</ItemGroup>
</Project>
113 changes: 113 additions & 0 deletions src/tests/Loader/CollectibleAssemblies/Statics/Unloaded.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
// 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;

// Use multiple classes to trigger multiple statics allocation events
public class StaticTest2
{
[ThreadStatic]
public static int ThreadStaticValue;
public static int StaticValue;
}

public class StaticTest3
{
public static int StaticValue;
}

public class StaticTest4
{
[ThreadStatic]
public static object ThreadStaticValue;
public static object StaticValue;
}
public class StaticTest5
{
[ThreadStatic]
public static object ThreadStaticValue;
public static object StaticValue;
}
public class StaticTest6
{
public static object StaticValue;
}


public class StaticTest : IStaticTest
{
[ThreadStatic]
public static int ThreadStaticValue;
public static int StaticValue;

[MethodImpl(MethodImplOptions.NoInlining)]
public static void SetValues(out int valTargetA, int valA
, out int valTargetB, int valB
, out int valTargetC, int valC
, out int valTargetD, int valD
, out int valTargetE, int valE
)
{
valTargetA = valA;
valTargetB = valB;
valTargetC = valC;
valTargetD = valD;
valTargetE = valE;
}

public void SetStatic(int val, int val2, int val3, int val4, int val5)
{
// Use this odd pathway to increase the chance that in the presence of GCStress issues will be found
SetValues(out ThreadStaticValue, val
, out StaticValue, val2
, out StaticTest2.StaticValue, val3
, out StaticTest2.ThreadStaticValue, val4
, out StaticTest3.StaticValue, val5
);
}

public void GetStatic(out int val1, out int val2, out int val3, out int val4, out int val5)
{
val1 = ThreadStaticValue;
val2 = StaticValue;
val3 = StaticTest2.StaticValue;
val4 = StaticTest2.ThreadStaticValue;
val5 = StaticTest3.StaticValue;
}

[MethodImpl(MethodImplOptions.NoInlining)]
public static void SetValuesObject(out object valTargetA, object valA
, out object valTargetB, object valB
, out object valTargetC, object valC
, out object valTargetD, object valD
, out object valTargetE, object valE
)
{
valTargetA = valA;
valTargetB = valB;
valTargetC = valC;
valTargetD = valD;
valTargetE = valE;
}

public void SetStaticObject(object val, object val2, object val3, object val4, object val5)
{
// Use this odd pathway to increase the chance that in the presence of GCStress issues will be found
SetValuesObject(out StaticTest4.ThreadStaticValue, val
, out StaticTest4.StaticValue, val2
, out StaticTest5.StaticValue, val3
, out StaticTest5.ThreadStaticValue, val4
, out StaticTest6.StaticValue, val5
);
}

public void GetStaticObject(out object val1, out object val2, out object val3, out object val4, out object val5)
{
val1 = StaticTest4.ThreadStaticValue;
val2 = StaticTest4.StaticValue;
val3 = StaticTest5.StaticValue;
val4 = StaticTest5.ThreadStaticValue;
val5 = StaticTest6.StaticValue;
}
}
12 changes: 12 additions & 0 deletions src/tests/Loader/CollectibleAssemblies/Statics/Unloaded.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<OutputType>Library</OutputType>
<CLRTestKind>BuildOnly</CLRTestKind>
<CLRTestPriority>0</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<ProjectReference Include="Accessor.csproj" />
<Compile Include="Unloaded.cs" />
</ItemGroup>
</Project>