[interp] Use existing InterpMethod if allocation and lookup race

[lambdageek]

If two threads both want to get an InterpMethod for the same MonoMethod, and they both see null from the first hash table lookup, make sure that whichever one comes into the jit_mm lock second re-uses the previously inserted InterpMethod, instead of its own version.

Without this change, racing threads will overwrite MonoJitInfo:seq_points (in mono_interp_transform_method) which sometimes leads to deallocating the same sequence points multiple times.

Fixes #57812

[ghost]

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

Issue Details

---

If two threads both want to get an InterpMethod for the same MonoMethod, and they both see null from the first hash table lookup, make sure that whichever one comes into the jit_mm lock second re-uses the previously inserted InterpMethod, instead of its own version.

Without this change, racing threads will overwrite MonoJitInfo:seq_points (in mono_interp_transform_method) which sometimes leads to deallocating the same sequence points multiple times.

Fixes #57812

Author:	lambdageek
Assignees:	-
Labels:	area-Codegen-Interpreter-mono
Milestone:	-

[lambdageek]

With this change running the test from #57812 in a loop doesn't lead to a crash after 20 minutes of runs. (before this change, I could get a crash in under a minute)

[lambdageek]

I'm a little bit unclear on why this is enough to fix the double-free. It's something about the MonoJitInfo getting created multiple times.

I would have expected that in mono_interp_transform_method we should only insert the seq_points data only on the thread that sets imethod->transformed to TRUE. But it seems like that's not necessary

runtime/src/mono/mono/mini/interp/transform.c

Lines 9821 to 9845 in 331cfe3

	if (!imethod->transformed) {
	// Ignore the first two fields which are unchanged. next_jit_code_hash shouldn't
	// be modified because it is racy with internal hash table insert.
	const int start_offset = 2 * sizeof (gpointer);
	memcpy ((char*)imethod + start_offset, (char*)&tmp_imethod + start_offset, sizeof (InterpMethod) - start_offset);
	mono_memory_barrier ();
	imethod->transformed = TRUE;
	mono_interp_stats.methods_transformed++;
	mono_atomic_fetch_add_i32 (&mono_jit_stats.methods_with_interp, 1);
	}
	mono_os_mutex_unlock (&calc_section);
	if (mono_stats_method_desc && mono_method_desc_full_match (mono_stats_method_desc, imethod->method)) {
	g_printf ("Printing runtime stats at method: %s\n", mono_method_get_full_name (imethod->method));
	mono_runtime_print_stats ();
	}
	MonoJitMemoryManager *jit_mm = get_default_jit_mm ();
	jit_mm_lock (jit_mm);
	gpointer seq_points = g_hash_table_lookup (jit_mm->seq_points, imethod->method);
	if (!seq_points || seq_points != imethod->jinfo->seq_points)
	g_hash_table_replace (jit_mm->seq_points, imethod->method, imethod->jinfo->seq_points);
	jit_mm_unlock (jit_mm);

[lambdageek]

/backport to release/6.0-rc1

[github-actions]

Started backporting to release/6.0-rc1: https://github.com/dotnet/runtime/actions/runs/1160620455