Guard MarshalByRefObject, [Serializable], and InitializeLifetimeService behind #if NETFRAMEWORK#8174
Open
Evangelink wants to merge 2 commits into
Open
Guard MarshalByRefObject, [Serializable], and InitializeLifetimeService behind #if NETFRAMEWORK#8174Evangelink wants to merge 2 commits into
Evangelink wants to merge 2 commits into
Conversation
…ce behind #if NETFRAMEWORK - Make all MarshalByRefObject inheritance conditional on NETFRAMEWORK across 10 classes - Wrap InitializeLifetimeService overrides and [SecurityCritical] in #if NETFRAMEWORK - Guard [Serializable] with #if NETFRAMEWORK on 11 non-exception data types - Conditionally include 'using System.Security' only for NETFRAMEWORK - Exception types retain [Serializable] unconditionally (standard .NET convention) On modern .NET (net8.0/net9.0), these classes are now plain types with no remoting or BinaryFormatter baggage. On net462, the full AppDomain marshaling support is preserved.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR scopes .NET Framework remoting/serialization artifacts (MarshalByRefObject, [Serializable], and InitializeLifetimeService) to #if NETFRAMEWORK, keeping AppDomain marshaling behavior on net462 while removing remoting-related baggage from modern .NET builds.
Changes:
- Guarded
MarshalByRefObjectinheritance andInitializeLifetimeService()overrides behind#if NETFRAMEWORK. - Guarded
[Serializable]on non-exception transport/data types behind#if NETFRAMEWORK. - Conditionally included
using System.Security;only where[SecurityCritical]is compiled to avoid unused-using warnings.
Show a summary per file
| File | Description |
|---|---|
| src/TestFramework/TestFramework/Attributes/TestMethod/TestResult.cs | Makes TestResult [Serializable] only on .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/RecursiveDirectoryPath.cs | Guards [Serializable], MarshalByRefObject, InitializeLifetimeService, and System.Security usage to .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/UnitTestElement.cs | Guards [Serializable] to .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/TestMethod.cs | Guards [Serializable] to .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/TestAssemblySettings.cs | Guards [Serializable] to .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/StackTraceInformation.cs | Guards [Serializable] to .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/NopTraceLogger.cs | Makes MarshalByRefObject base conditional for .NET Framework remoting. |
| src/Adapter/MSTestAdapter.PlatformServices/MSTestSettings.cs | Guards [Serializable] to .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Helpers/ReflectHelper.cs | Makes MarshalByRefObject/InitializeLifetimeService and System.Security usage conditional on .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/UnitTestRunner.cs | Makes MarshalByRefObject/InitializeLifetimeService and System.Security usage conditional on .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TypeCache.cs | Makes MarshalByRefObject/InitializeLifetimeService and System.Security usage conditional on .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TestExecutionManager.cs | Makes nested RemotingMessageLogger inherit MarshalByRefObject only on .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TestAssemblySettingsProvider.cs | Makes MarshalByRefObject/InitializeLifetimeService and System.Security usage conditional on .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Discovery/AssemblyEnumerator.cs | Makes MarshalByRefObject/InitializeLifetimeService and System.Security usage conditional on .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Discovery/AssemblyEnumerationResult.cs | Guards [Serializable] to .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Deployment/TestRunDirectories.cs | Guards [Serializable] to .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Deployment/DeploymentItem.cs | Guards [Serializable] on enum and class to .NET Framework. |
| src/Adapter/MSTest.TestAdapter/VSTestAdapter/EqtTraceLogger.cs | Makes MarshalByRefObject base conditional for .NET Framework remoting. |
| src/Adapter/MSTest.TestAdapter/TestingPlatformAdapter/MTPTraceLogger.cs | Makes MarshalByRefObject base conditional for .NET Framework remoting. |
Copilot's findings
- Files reviewed: 19/19 changed files
- Comments generated: 0
Youssef1313
reviewed
May 13, 2026
Comment on lines
+11
to
+13
| #if NETFRAMEWORK | ||
| [Serializable] | ||
| #endif |
Member
There was a problem hiding this comment.
Consider reverting and keep the changes only in Adapter, or at least have this under #if NETFRAMEWORK || NETSTANDARD to be safe. I would be worried if TestFramework netstandard assemblies end up getting loaded in a .NET Framework process where Serializable will be needed.
Under adapter, we no longer target .NET Standard so we are pretty safe there.
Member
There was a problem hiding this comment.
Actually better to simply just revert in this case.
This is a public class and we might not know what users are doing with it.
Member
Author
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Contributor
Resolved. The only conflict was in |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Audit and clean up
MarshalByRefObjectand[Serializable]usage acrosssrc/to ensure these .NET Framework remoting artifacts are properly scoped tonet462only.Changes
MarshalByRefObjectinheritance (10 classes)Made all
MarshalByRefObjectbase class inheritance conditional on#if NETFRAMEWORK:Classes with only MarshalByRefObject as base (6):
AssemblyEnumerator,ReflectHelper,RecursiveDirectoryPath,UnitTestRunner,TypeCache,TestAssemblySettingsProviderClasses implementing MarshalByRefObject + an interface (4):
RemotingMessageLogger,NopTraceLogger,EqtTraceLogger,MTPTraceLoggerInitializeLifetimeServiceoverrides (6 methods)Wrapped all
InitializeLifetimeService()overrides (including[SecurityCritical]and the previous[Obsolete]attributes) entirely in#if NETFRAMEWORK. Onnet462, the override returnsnullfor infinite lifetime. On modern .NET, the override doesn't exist since the base class is no longerMarshalByRefObject.Also conditionally included
using System.Security;to avoid IDE0005 warnings.[Serializable]attribute (11 non-exception types)Guarded
[Serializable]with#if NETFRAMEWORKon data types that only needed it forBinaryFormattermarshaling across AppDomain boundaries:RecursiveDirectoryPath,UnitTestElement,TestMethod,TestAssemblySettings,StackTraceInformation,MSTestSettings,AssemblyEnumerationResult,TestRunDirectories,DeploymentItem,DeploymentItemOriginType,TestResultException types (
AssertFailedException,UnitTestAssertException, etc.) intentionally retain[Serializable]unconditionally — this is a standard .NET convention for exceptions.Already correctly guarded (no changes needed)
These were already behind
#if NETFRAMEWORK:AssemblyLoadWorker,StaticStateHelper(inAppDomainUtilities.cs),AssemblyResolver(conditional base class)AppDomain.CreateDomain/Unloadcalls inAppDomainUtilities.cs,AppDomainWrapper.cs,IAppDomain.cs,TestSourceHost.csResult
On modern .NET (
net8.0/net9.0), these classes are now plain types with no remoting orBinaryFormatterbaggage. Onnet462, the full AppDomain marshaling support is preserved.Validation
dotnet buildnet9.0 — 0 warnings, 0 errorsdotnet buildnet462 — 0 warnings, 0 errorsdotnet buildMSTest.TestAdapter net9.0 — 0 warnings, 0 errorsdotnet buildTestFramework net9.0 — 0 warnings, 0 errors