Race condition on marshal-ilgen init

[naricc]

Description

Because of this change: #71203, mono_marshal_ilgen_init gets called later, after comnponents are initilized. This allows for potential races in initilizing the callback table.

I propose to fix this by making a pointer to ilgen_marshal_cb, and initing that pointer atomically (with a CAS) after the table has been constructed.

See comment here: #71203 (comment)

Reproduction Steps

Race condition; not easily reproducible.

Expected behavior

Runs

Actual behavior

Assertion at /home/uweigand/runtime/src/mono/mono/component/marshal-ilgen.c:63, condition `!ilgen_cb_inited' not met

with this call stack:

  #14 0x000003ffb327d0ac in mono_assertion_message (file=0x3ffb33865a0 "/home/uweigand/runtime/src/mono/mono/component/marshal-ilgen.c", line=63, condition=0x3ffb33865e0 "!ilgen_cb_inited") at /home/uweigand/runtime/src/mono/mono/eglib/goutput.c:226
  #15 0x000003ffb3321a30 in mono_install_marshal_callbacks_ilgen (cb=0x3ff9a3f16d0) at /home/uweigand/runtime/src/mono/mono/component/marshal-ilgen.c:63
  #16 0x000003ffb33221ce in mono_marshal_ilgen_init () at /home/uweigand/runtime/src/mono/mono/component/marshal-ilgen.c:2858
  #17 0x000003ffb3322106 in get_marshal_cb () at /home/uweigand/runtime/src/mono/mono/component/marshal-ilgen.c:2753
  #18 0x000003ffb3321f24 in mono_emit_marshal_ilgen (m=0x3ff9a3f1bc8, argnum=0, t=0x3ff8ad98b68, spec=0x0, conv_arg=0, conv_arg_type=0x3ff8ad99ed8, action=MARSHAL_ACTION_CONV_IN, lightweigth_cb=0x3ffb33910e8 <marshal_lightweight_cb>) at /home/uweigand/runtime/src/mono/mono/component/marshal-ilgen.c:2810
  #19 0x000003ffb2e54c52 in mono_emit_marshal (m=0x3ff9a3f1bc8, argnum=0, t=0x3ff8ad98b68, spec=0x0, conv_arg=0, conv_arg_type=0x3ff8ad99ed8, action=MARSHAL_ACTION_CONV_IN) at /home/uweigand/runtime/src/mono/mono/metadata/marshal.c:3226
  #20 0x000003ffb2f219aa in emit_native_wrapper_ilgen (image=0x2aa18a8b950, mb=0x3ff7c90eb50, sig=0x3ff8ad99cc8, piinfo=0x3ff8ad98af0, mspecs=0x3ff7c8f47e0, func=0x3ff82d81750 <CompressionNative_DeflateInit2_>, flags=(EMIT_NATIVE_WRAPPER_CHECK_EXCEPTIONS | EMIT_NATIVE_WRAPPER_RUNTIME_MARSHALLING_ENABLED)) at /home/uweigand/runtime/src/mono/mono/metadata/marshal-lightweight.c:937
  #21 0x000003ffb2e5755a in mono_marshal_emit_native_wrapper (image=0x2aa18a8b950, mb=0x3ff7c90eb50, sig=0x3ff8ad99cc8, piinfo=0x3ff8ad98af0, mspecs=0x3ff7c8f47e0, func=0x3ff82d81750 <CompressionNative_DeflateInit2_>, flags=(EMIT_NATIVE_WRAPPER_CHECK_EXCEPTIONS | EMIT_NATIVE_WRAPPER_RUNTIME_MARSHALLING_ENABLED)) at /home/uweigand/runtime/src/mono/mono/metadata/marshal.c:6300
  #22 0x000003ffb2e56b6c in mono_marshal_get_native_wrapper (method=0x3ff8ad98af0, check_exceptions=1, aot=0) at /home/uweigand/runtime/src/mono/mono/metadata/marshal.c:3672
  #23 0x000003ffb30328e8 in compile_special (method=0x3ff8ad98af0, error=0x3ff9a3f2c38) at /home/uweigand/runtime/src/mono/mono/mini/mini-runtime.c:2469
  #24 0x000003ffb30322b0 in mono_jit_compile_method_with_opt (method=0x3ff8ad98af0, opt=374417919, jit_only=0, error=0x3ff9a3f2c38) at /home/uweigand/runtime/src/mono/mono/mini/mini-runtime.c:2672
  #25 0x000003ffb3031c04 in jit_compile_method_with_opt_cb (arg=0x3ff9a3f28e8) at /home/uweigand/runtime/src/mono/mono/mini/mini-runtime.c:2762
  #26 0x000003ffb3029ea8 in jit_compile_method_with_opt (params=0x3ff9a3f28e8) at /home/uweigand/runtime/src/mono/mono/mini/mini-runtime.c:2778
  #27 0x000003ffb3029c7e in mono_jit_compile_method (method=0x3ff8ad98af0, error=0x3ff9a3f2c38) at /home/uweigand/runtime/src/mono/mono/mini/mini-runtime.c:2797
  #28 0x000003ffb31677ba in common_call_trampoline (regs=0x3ff9a3f2db8, code=0x3ffa806234c "\a\340\300\001", m=0x3ff8ad98af0, vt=0x0, vtable_slot=0x0, error=0x3ff9a3f2c38) at /home/uweigand/runtime/src/mono/mono/mini/mini-trampolines.c:618
  #29 0x000003ffb3166b8e in mono_magic_trampoline (regs=0x3ff9a3f2db8, code=0x3ffa806234c "\a\340\300\001", arg=0x3ff8ad98af0, tramp=0x3ffb3de02c8 "\300X") at /home/uweigand/runtime/src/mono/mono/mini/mini-trampolines.c:759

Regression?

No response

Known Workarounds

No response

Configuration

ppc, but probably possible on other architectures

Other information

No response

[naricc]

@uweigand I've created this issues based on your comment from the PR. Thanks for finding that.

Is this blocking you? If it is an immediate blocker I will jump on this right now, otherwise I will wait until early next week.

[uweigand]

No, it's not a blocker - I just saw this on one testsuite run, and haven't been able to reproduce since.

Thanks for looking into this!

[SamMonoRT]

cc @lambdageek fyi

[SamMonoRT]

reverted the original change from 7.0. Moving this to 8.0

[lambdageek]

#77090 seems like the same issue - the race is there even before the componentization work. It's worth fixing and backporting.

[SamMonoRT]

@naricc - we should add the fix for this in your existing PR for completeness

[naricc]

Working on taking the fix from the marshal-component change and applying it independent of that change, so we can easily backport it to 7.

[jandupej]

Backporting to 7.0 in #83813