Improve logging for AppDomain assembly load tracking

[OvesN]

Fixes #13254

Context

When an AppDomainIsolatedTask (for example, SignToolTask) fails in a child AppDomain on .NET Framework, AssemblyLoadsTracker can mask the real task failure with a SerializationException.

The root cause is that the tracker used to subscribe to AppDomain.AssemblyLoad on the child AppDomain from the parent AppDomain. When a load happened in the child AppDomain, AssemblyLoadEventArgs had to cross the AppDomain boundary. AssemblyLoadEventArgs is not [Serializable], so marshaling it across the boundary threw SerializationException, which replaced the original task failure.

This was observed in the Windows Full Release (no bootstrap) pipeline, where the signing path surfaced SerializationException instead of the underlying file/path failure.

This error masking was not oberved in VMR pipline because it uses .Core which does not implements AppDomain so cross domain communication cannot happen.

Changes made

AssemblyLoadsTracker.cs

• Skip assembly-load tracking when the target AppDomain is not AppDomain.CurrentDomain under #if FEATURE_APPDOMAIN.
• Return EmptyDisposable.Instance for child-AppDomain tracking requests.

This means we now avoid all cross-AppDomain AssemblyLoad event subscriptions and unsubscriptions for this scenario.

Same-domain tracking behavior is unchanged.

Tests

added unit tests that demonstrated the original child-AppDomain behavior: assembly loads in the child AppDomain were not logged, and a SerializationException was occurring underneath during cross-AppDomain event dispatch. See commit 66f6ad1

I was not able to create a unit test that directly reproduces the original masking behavior.
For end-to-end validation, I temporarily modified the Windows Full Release (no bootstrap) pipeline—the pipeline where the masked signing failure had previously been observed—to run the signing path with bootstrapped MSBuild from this branch instead of the VS MSBuild normally used there.

With that temporary validation-only change, the pipeline surfaced the real System.IO.DirectoryNotFoundException instead of the masking SerializationException.

[Copilot]

Pull request overview

This PR prevents AssemblyLoadsTracker from masking real task failures with SerializationException when tasks run in a separate (child) AppDomain on .NET Framework by avoiding cross-AppDomain subscription to AppDomain.AssemblyLoad.

Changes:

• Add a FEATURE_APPDOMAIN-gated guard to skip subscribing to AssemblyLoad when tracking a non-current AppDomain.
• Preserve existing behavior for the current AppDomain and for .NET Core (single AppDomain).

Comments suppressed due to low confidence (1)

src/Build/BackEnd/Components/RequestBuilder/AssemblyLoadsTracker.cs:156

• The new guard returns early for remote AppDomains, but the tracker instance is still created/registered (via the static StartTracking path) and StopTracking() will still do _appDomain.AssemblyLoad -= .... This makes AssemblyLoadsTracker.StopTracking(appDomain) perform an unnecessary cross-AppDomain call for a tracker that never subscribed (and could potentially throw if the AppDomain has already been unloaded). Consider either (1) moving the remote-AppDomain check into the static StartTracking(...) method so it returns EmptyDisposable and doesn’t add to s_instances, or (2) tracking a _subscribed flag and only unsubscribing when subscription actually occurred.

#if FEATURE_APPDOMAIN
            if (_appDomain != AppDomain.CurrentDomain)
            {
                // Subscribing to AssemblyLoad on a remote AppDomain causes the event handler to be
                // invoked through a transparent proxy. AssemblyLoadEventArgs is not [Serializable],
                // so marshaling it across the AppDomain boundary throws SerializationException.
                // That exception propagates through ITask.Execute() and masks the real task error.
                // Skip assembly load tracking for tasks running in separate AppDomains.
                return;
            }
#endif
            _appDomain.AssemblyLoad += CurrentDomainOnAssemblyLoad;
        }

        private void StopTracking()
        {
            _appDomain.AssemblyLoad -= CurrentDomainOnAssemblyLoad;

[YuliiaKovalova]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[YuliiaKovalova]

Nice find on the root cause - the SerializationException masking the real task error is a nasty issue.
A couple of things I noticed with the current approach:

1. Dead trackers in s_instances The guard bails out of the instance StartTracking(), but the static StartTracking(...) has already added the tracker to s_instances (line 126–131). So you end up with tracker objects sitting in the static list that never subscribed to anything - they're just dead weight until StopTracking(AppDomain) cleans them up.

2. StopTracking() unsubscribes across the AppDomain boundary When Dispose() or StopTracking(AppDomain) runs, it calls _appDomain.AssemblyLoad -= CurrentDomainOnAssemblyLoad - which is a cross-domain call on the remote AppDomain. The -= on a never-subscribed handler is a no-op and likely won't throw, but if that AppDomain has already been unloaded by the time cleanup runs, you could get an AppDomainUnloadedException.

[YuliiaKovalova]

approve with the 2 minor comments

[JanKrivanek]

Can we wrap the CurrentDomainOnAssemblyLoad with exception handling - and so keep the loads logged in normal case, but be able to recover in error case?

[JanKrivanek]

Great job!

[OvesN]

Can we wrap the CurrentDomainOnAssemblyLoad with exception handling - and so keep the loads logged in normal case, but be able to recover in error case?

This would not help. Using the tests from commit 66f6ad1 I verified that when cross-AppDomain communication is involved, CurrentDomainOnAssemblyLoad is not entered at all.