runtime

runtime copied to clipboard

If we memset a vt containing references, then, in order for preemptive GC and concurrent GC to work, writing to reference slots must happen atomically, otherwise GC might encounter a garbage ref.

Also speeds up memset by about 15% for smaller valuetypes hitting this path on 64bit. More for extra large valuetypes.

Fixes https://github.com/dotnet/runtime/issues/58828

Tagging subscribers to this area: @brzvlad See info in area-owners.md if you want to be subscribed.

Issue Details

---

If we memset a vt containing references, then, in order for preemptive GC and concurrent GC to work, writing to reference slots must happen atomically, otherwise GC might encounter a garbage ref.

Also speeds up memset by about 15% for smaller valuetypes hitting this path on 64bit. More for extra large valuetypes.

Fixes https://github.com/dotnet/runtime/issues/58828

Author:	BrzVlad
Assignees:	BrzVlad
Labels:	area-CoreLib-mono, area-GC-mono
Milestone:	-

This bug can be easily reproduced in a matter of seconds with the following test case:

using System;
using System.Threading;

public struct VTWithRefs {
	public object o0, o1, o2, o3, o4, o5, o6, o7, o8, o9, o10;
}

public class Program {
	public static Program p;

	public static void GCTrigger () {
		int index = 0;
		int n = 100000;
		Program[] major_objs = new Program [n];
		while (true) {
			major_objs [index] = new Program ();
			index++;
			if (index == n)
				index = 0;
		}
	}

	public static void Main (string[] args)
	{
		const int n = 1000;
		VTWithRefs[] array = new VTWithRefs [n];
		string obj = "obj";

		Thread thread = new Thread (GCTrigger);
		thread.Start ();

		while (true) {
			for (int i = 0; i < n; i++) {
				array [i] = default (VTWithRefs);
				array [i].o0 = obj;
				array [i].o1 = obj;
				array [i].o2 = obj;
				array [i].o3 = obj;
				array [i].o4 = obj;
				array [i].o5 = obj;
				array [i].o6 = obj;
				array [i].o7 = obj;
				array [i].o8 = obj;
				array [i].o9 = obj;
			}
		}
	}
}

/azp run runtime-extra-platforms

Azure Pipelines successfully started running 1 pipeline(s).

Should we backport this to 6.0?

@akoeplinger I think we should

/backport to release/6.0

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/2835143319

@akoeplinger backporting to release/6.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: [mono][jit] Don't memset memory in increments lower than word size
Using index info to reconstruct a base tree...
M	src/mono/System.Private.CoreLib/src/System/String.Mono.cs
Falling back to patching base and 3-way merge...
Auto-merging src/mono/System.Private.CoreLib/src/System/String.Mono.cs
CONFLICT (content): Merge conflict in src/mono/System.Private.CoreLib/src/System/String.Mono.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 [mono][jit] Don't memset memory in increments lower than word size
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@BrzVlad please backport manually