Skip to content

[Bug]: In Release builds every dependency inside an async method is lost, so negative rules pass vacuously (state machine found via first newobj) #498

Description

@remihenache

Description

In an optimized build (-c Release), every dependency written inside an async method is missing from the architecture. That's any call, any new, any field access, not only the "an async function is called" case listed in Debug Artifacts / LimitationsOnReleaseTest.AsyncMethodDependencyTest.

The dangerous consequence is on negative rules. NotDependOnAny / NotCallAny pass for a violation written inside an async method, so a Release test run reports green on code that breaks the rule. Most application code in a modern .NET codebase is async, so this silently switches off much of what a layering rule protects. Positive rules (DependOnAny, CallAny) fail instead, as reported in #326.

I think the cause is a heuristic in the loader rather than something the optimizer inherently hides, and it looks fixable (proposal below).

Root cause. HandleAsync (TypeProcessor.HandleAsync in 0.13.4; AddMethodDependencies.HandleAsync in 0.11.x) finds the state machine by taking the declaring type of the first newobj in the kickoff method:

  • Debug: the state machine is a class, and the kickoff starts with newobj <RunAsync>d__0::.ctor(). Works.
  • Release: the state machine is a struct. The kickoff does ldloca.s 0 / stfld on a local of that type, and there is no newobj. HandleAsync falls back to methodDefinition = methodBody.Method, so only the stub is analysed and MoveNext never is.

Proposed fix. Resolve the state machine from the attribute the compiler already emits in both configurations, [AsyncStateMachine(typeof(<RunAsync>d__0))]: CustomAttributes[AsyncStateMachineAttribute].ConstructorArguments[0].Value is the state machine's TypeReference, whether it's a class or a struct. The last test in the MWE below does exactly that with Mono.Cecil, and it finds the call in MoveNext in both Debug and Release. The same could apply to HandleIterator / IteratorStateMachineAttribute for robustness, although iterator state machines are classes in both configurations today.

Minimal Working Example

// net10.0 test project: TngTech.ArchUnitNET.xUnit 0.13.4, Mono.Cecil 0.11.6, xunit 2.9.3
using System.Runtime.CompilerServices;
using ArchUnitNET.Domain;
using ArchUnitNET.Loader;
using ArchUnitNET.xUnit;
using Mono.Cecil;
using Mono.Cecil.Cil;
using Xunit;
using static ArchUnitNET.Fluent.ArchRuleDefinition;

namespace Repro
{
    public static class Forbidden { public static void Touch() { } }

    public class SyncCaller
    {
        public void Run() => Forbidden.Touch();
    }

    public class AsyncCaller
    {
        // A plain SYNCHRONOUS call, merely written inside an async method.
        public async Task RunAsync()
        {
            await Task.Yield();
            Forbidden.Touch();
        }
    }

    public class ReleaseDropsAsyncBodies
    {
        private static readonly Architecture Architecture =
            new ArchLoader().LoadAssembly(typeof(ReleaseDropsAsyncBodies).Assembly).Build();

        [Fact] // control: passes in Debug and Release
        public void Sync_caller_is_seen() =>
            Classes().That().Are(typeof(SyncCaller)).Should().DependOnAny(typeof(Forbidden)).Check(Architecture);

        [Fact] // passes in Debug, FAILS in Release
        public void Async_caller_is_seen() =>
            Classes().That().Are(typeof(AsyncCaller)).Should().DependOnAny(typeof(Forbidden)).Check(Architecture);

        [Fact] // the consequence: the negative rule fails in Debug (correct) but PASSES in Release (vacuous)
        public void A_negative_rule_catches_the_async_caller() =>
            Assert.ThrowsAny<Exception>(() =>
                Classes().That().Are(typeof(AsyncCaller)).Should().NotDependOnAny(typeof(Forbidden)).Check(Architecture));

        [Fact] // the proposed fix path: the attribute names the state machine in BOTH configurations
        public void The_attribute_leads_to_MoveNext_in_any_configuration()
        {
            using var module = ModuleDefinition.ReadModule(typeof(AsyncCaller).Assembly.Location);
            var kickoff = module.GetType(typeof(AsyncCaller).FullName).Methods.Single(m => m.Name == "RunAsync");
            var attribute = kickoff.CustomAttributes.Single(a => a.AttributeType.FullName == typeof(AsyncStateMachineAttribute).FullName);
            var stateMachine = ((TypeReference)attribute.ConstructorArguments[0].Value).Resolve();
            var moveNext = stateMachine.Methods.Single(m => m.Name == "MoveNext");

            Assert.Contains(moveNext.Body.Instructions, i =>
                i.OpCode == OpCodes.Call && i.Operand is MethodReference m && m.Name == nameof(Forbidden.Touch));
            Console.WriteLine($"state machine is a {(stateMachine.IsValueType ? "struct" : "class")}; " +
                              $"kickoff has newobj: {kickoff.Body.Instructions.Any(i => i.OpCode == OpCodes.Newobj)}");
        }
    }
}

Run with dotnet test -c Debug and dotnet test -c Release.

Expected Behavior

Both configurations produce the same dependencies for AsyncCaller: all four tests pass under -c Debug and -c Release.

Actual Behavior

test Debug Release
Sync_caller_is_seen ✅ ✅
Async_caller_is_seen ✅ ❌
A_negative_rule_catches_the_async_caller ✅ ❌ (the NotDependOnAny rule passed)
The_attribute_leads_to_MoveNext_in_any_configuration ✅ ✅

The Release failure:

ArchUnitNET.xUnit.FailedArchRuleException : "Classes that are "Repro.AsyncCaller" should depend on "Repro.Forbidden"" failed:
	Repro.AsyncCaller does not depend on any type

Diagnostic output from the last test:

Debug:   state machine is a class;  kickoff has newobj: True
Release: state machine is a struct; kickoff has newobj: False

ArchUnitNET Version

0.13.4 (also reproduced on 0.11.4)

.NET Version

.NET 10.0 (SDK 10.0.204)

Additional Context

  • Related: Tests may behave differently in Release configuration #326, which reported the positive-rule side of this and was resolved by documenting the limitation. This issue adds the vacuous negative-rule side, identifies the root cause as the newobj lookup, and proposes a fix that appears to work in both configurations.
  • The workaround we use: point the architecture tests at unoptimized builds of the analysed projects whatever configuration the test run uses, and refuse optimized assemblies (DebuggableAttribute.IsJITOptimizerDisabled == false) before loading them, so a Release run can't report a pass on a graph missing its async bodies.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-triageIndicates that an issue needs to be categorized.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions