Fix init race in mono_class_try_get_[shortname]_class. (#112296)

Backport of #112282 to release/9.0

We observed several crash reports in dotnet Android coming from
google play store, like this one:

#109921 (comment)

and it happens at the following callstack:

mono_class_setup_vtable_full at /__w/1/s/src/mono/mono/metadata/class-setup-vtable.c:900
init_io_stream_slots at /__w/1/s/src/mono/mono/metadata/icall.c:3378
ves_icall_System_IO_Stream_HasOverriddenBeginEndRead at /__w/1/s/src/mono/mono/metadata/icall.c:3437
 (inlined by) ves_icall_System_IO_Stream_HasOverriddenBeginEndRead_raw at /__w/1/s/src/mono/mono/metadata/../metadata/icall-def.h:228

Looking a little deeper at that that code path expects call to
mono_class_try_get_stream_class to succeed since it calls
mono_class_setup_vtable on returned class pointer. There are other
places where code expectes mono_class_try_get_[shortname]_class to
always succeed or it will assert. Other locations handles the case
where it returns NULL meaning that the class has been linked out.

After looking into the macro implementation generating the
mono_class_try_get_[shortname]_class functions it appears that it has
a race where one thread could return NULL even if the class successfully
gets loaded by another racing thread. Initially that might have
been intentionally and callers would need to redo the call, but then
there is no way for callers to distinct between hitting the race and
class not available. Adding to the fact that code would also expect that
some critical classes never fail loading, like what
init_io_stream_slots does, this race ended up in race conditions.

In the past, this particular race in init_io_stream_slots was hidden
due to multiple calls to mono_class_try_get_stream_class minimzing the
risk of a race to cause a crash. That implementation was however
changed by e74da7c
ending up in just one call to mono_class_try_get_stream_class in the
critical paths.

Since code relies on mono_class_try_get_[shortname]_class to succeed
for some critical classes, code asserts if it returns NULL or crashes.
The fix will use acquire/release semantics on the static variable
guarding the cached type and make sure memory order is preserved
on both read and write. Previous implementation adopted a similar approach
but it took a copy of the static tmp_class into a local variable meaning
that memory order would not be guaranteed, even if it applied memory
barriers and volatile keywords on the static variables.

Fix also adds an assert into init_io_stream_slots since it expects
failures in calls to mono_class_try_get_stream_class as fatal.

Author: github-actions[bot]

src/mono/mono/metadata/class-internals.h

@@ -952,25 +952,20 @@ mono_class_get_##shortname##_class (void)	\
 
 // GENERATE_TRY_GET_CLASS_WITH_CACHE attempts mono_class_load_from_name approximately
 // only once. i.e. if it fails, it will return null and not retry.
-// In a race it might try a few times, but not indefinitely.
-//
-// FIXME This maybe has excessive volatile/barriers.
-//
 #define GENERATE_TRY_GET_CLASS_WITH_CACHE(shortname,name_space,name) \
 MonoClass*	\
 mono_class_try_get_##shortname##_class (void)	\
 {	\
-	static volatile MonoClass *tmp_class;	\
-	static volatile gboolean inited;	\
-	MonoClass *klass = (MonoClass *)tmp_class;	\
-	mono_memory_barrier ();	\
-	if (!inited) {	\
-		klass = mono_class_try_load_from_name (mono_class_generate_get_corlib_impl (), name_space, name);	\
-		tmp_class = klass;	\
-		mono_memory_barrier ();	\
-		inited = TRUE;	\
+	static MonoClass *cached_class;	\
+	static gboolean cached_class_inited;	\
+	gboolean tmp_inited;	\
+	mono_atomic_load_acquire(tmp_inited, gboolean, &cached_class_inited);	\
+	if (G_LIKELY(tmp_inited)) {	\
+		return (MonoClass*)cached_class;	\
 	}	\
-	return klass;	\
+	cached_class = mono_class_try_load_from_name (mono_class_generate_get_corlib_impl (), name_space, name);	\
+	mono_atomic_store_release(&cached_class_inited, TRUE); \
+	return (MonoClass*)cached_class;	\
 }
 
 GENERATE_TRY_GET_CLASS_WITH_CACHE_DECL (safehandle)

src/mono/mono/metadata/icall.c

@@ -3391,6 +3391,8 @@ static void
 init_io_stream_slots (void)
 {
 	MonoClass* klass = mono_class_try_get_stream_class ();
+	g_assert(klass);
+
 	mono_class_setup_vtable (klass);
 	MonoMethod **klass_methods = m_class_get_methods (klass);
 	if (!klass_methods) {