runtime: Access Violation through reflection not caught as NRE on .NET 6

Description

For some reason, reading a simple tiny property via reflection that should NRE instead crashes the process with an AV (seems the runtime isn’t turning it into an NRE correctly?). This start happening with .NET 6 (we migrated two days ago, from .NET 5) and only seems to happen through reflection GetValue()/Invoke(). It’s also specific to release mode (debug works fine) and happens consistently on both Linux/Windows.

We have a debugging tool in our game called VV (view variables) which allows us to inspect and modify the fields/properties on objects. Outside of being networked (server connection) it just uses basic reflection stuff to find all properties/fields tagged with [ViewVariables] and uses .GetValue() to view their value, show them nicely, etc… We noticed that this hard crashed the server when looking at certain objects. Accessing the property directly (at least via C# interactive) works fine. Accessing it via reflection crashes (it’s not specific to our VV tool).

The problematic code (that’s being invoked and causes the crash) is this tiny property. The objects in question being looked at do have a null Owner, so an NRE is expected. Yeah, it’s that simple. There’s no IL rewriting, unsafe optimizations, anything going on.

Rickbrew on Discord suggested setting COMPlus_legacyCorruptedStateExceptionsPolicy=1. If I do that the AV gets thrown as AccessViolationException but can at least be caught with a try catch (it’s wrapped in an InvocationTargetException of course). I don’t know how relevant that is. Ræin on Discord suggested using a function pointer like so: ((delegate*<[YourObjectType], EntityUid>)property.GetMethod.Handle.GetFunctionPointer())(Object). This worked fine (NRE thrown and caught). Other things they suggested like property.GetMethod.Invoke(...) exhibit the same AV behavior.

Reproduction Steps

I tried reproducing this in a tiny test project and couldn’t get it to happen, in fact a separate test game project on our own (same) engine doesn’t even trigger it. It is consistent in our game however. I understand these aren’t very lightweight instructions but anyways:

Clone repo (also we have a setup guide in case it doesn’t work).
git submodule update --init --recursive
dotnet run -c Release in the Content.Client project
[edit] I just realized this appears to be related to optimization tiers or something, running a bad code sample straight away doesn’t trigger it. Going in game proper so the JIT warms up I guess and then running the code does.
dotnet run -c Release in the Content.Server project to start the server
Hit the big connect button on the client, wait for it to connect.
Open dev console (tilde/grave, below escape)
Run csi for C# interactive console.
Run the following code:

var c = new TransformComponent(); // Literally any component instance works here. These components have no constructors, nothing funny here.
c.GetType().GetProperty("OwnerUid").GetValue(c); // Will cause AV.

I tried debugging this in SOS but didn’t really know what I was doing all that much. Tell me if I need to do something specific). I can supply a dump if necessary.

Expected behavior

The runtime should simply throw an NRE which our code will handle (admittedly our VV code doesn’t handle it nicely but the packet handler catches it so it won’t crash the whole process like it does now).

Actual behavior

The runtime seems to fail to detect the AV as an NRE and crashes the process.

Regression?

Yes, I rolled back to .NET 5 (we switched .NET 6 two days ago) and it stopped happening. The exception is correctly thrown as an NRE and caught.

Known Workarounds

No response

Configuration

.NET Runtime: 6.0.0 OS: Windows 10.0.19042 for my dev machine, but it also happens on our Linux servers. Architecture: x64 Specific: Happens on Linux and Windows, does not happen on .NET 5, only happens on Release build configuration (not Debug).

Other information

No response

About this issue

Original URL
State: closed
Created 3 years ago
Reactions: 2
Comments: 19 (19 by maintainers)

Commits related to this issue

Add explicit null-check for tailcalls to VSD There is already a comment that this is necessary, but it is only being done for x86 tailcalls via jit helper. Do it for normal tailcalls to VSD as well. ... — committed to jakobbotsch/runtime by jakobbotsch 3 years ago
Add explicit null-check for tailcalls to VSD (#62719) There is already a comment that this is necessary, but it is only being done for x86 tailcalls via jit helper. Do it for normal tailcalls to VSD... — committed to dotnet/runtime by jakobbotsch 3 years ago
Add explicit null-check for tailcalls to VSD There is already a comment that this is necessary, but it is only being done for x86 tailcalls via jit helper. Do it for normal tailcalls to VSD as well. ... — committed to dotnet/runtime by jakobbotsch 3 years ago
[release/6.0] Add explicit null-check for tailcalls to VSD (#62769) * Add explicit null-check for tailcalls to VSD There is already a comment that this is necessary, but it is only being done for... — committed to dotnet/runtime by github-actions[bot] 2 years ago

Most upvoted comments

Here is a small repro - compile and run with optimizations on:

var i = new My(new My(null));
var m = i.GetType().GetMethod("M");
for (;;)
{
    try
    {
        m.Invoke(i, null);
    }
    catch
    {
    }
}

public interface IFace
{
    void M();
}

public class My : IFace
{
    IFace _face;

    public My(IFace face)
    {
        _face = face;
    }

    public void M() => _face.M();
}

Expected behavior: Runs forever Actual behavior: Crashes with AccessViolationException

jkotas on Nov 12, 2021