Replace contract factories with registration-based ContractRegistry by max-charlamb · Pull Request #126511 · dotnet/runtime

max-charlamb · 2026-04-03T17:49:28Z

Note

This PR was created with the assistance of GitHub Copilot.

Summary

Replace IContractFactory<T> and 26 named factory classes with a flat registration API: ContractRegistry.Register<T>(version, creator).

Motivation

The existing factory pattern requires modifying multiple files to add contract support for new runtimes (e.g., NativeAOT). Each contract has a dedicated factory class with a hardcoded version switch. Adding a new runtime means touching every factory.

Changes

New registration API

// Any package registers its contracts the same way:
public static void Register(ContractRegistry registry)
{
    registry.Register<IThread>(1, static t => new Thread_1(t));
    registry.Register<IRuntimeTypeSystem>(1, static t => new RuntimeTypeSystem_1(t));
    // ...
}

CachingContractRegistry

Zero contract knowledge — pure (Type, int) → creator registry
Takes params Action<ContractRegistry>[] for composable registration
Built-in CoreCLR and external packages use the same Register<T>() API

Contract constructors

Simplified to take only Target target
Each reads its own globals internally (Object_1, GC_1, Thread_1, etc.)

CoreCLRContracts.cs (new)

Single file registering all 26 CoreCLR contract versions
Same pattern external packages would use

Deleted (26 files)

All named factory classes (ThreadFactory, ExceptionFactory, ObjectFactory, etc.)

Enables

External contract packages (NativeAOT, Mono) without modifying the main cDAC reader
ContractDescriptorTarget.TryCreate(addr, ..., [CoreCLRContracts.Register, NativeAOTContracts.Register], out target)

Testing

All 1406 existing cDAC tests pass.

dotnet-policy-service · 2026-04-03T17:50:57Z

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

This PR refactors the cDAC contract instantiation model from per-contract IContractFactory<T> implementations to a registration-based approach, enabling composable contract providers (e.g., CoreCLR + external runtimes) without modifying the core reader.

Changes:

Introduced ContractRegistry.Register<T>(version, creator) and updated CachingContractRegistry to resolve contracts via (Type, version) -> creator.
Added CoreCLRContracts.Register to centralize CoreCLR contract registrations; removed a large set of named factory classes.
Updated target creation call sites (reader entrypoints + tests) to pass contract registration actions, and simplified contract constructors to take only Target.

Reviewed changes

Copilot reviewed 68 out of 68 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs	Adds `Register<TContract>` API to the registry abstraction.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs	Switches from factory map to `(type, version)` creator registration + caching.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs	Replaces `additionalFactories` with `contractRegistrations` during target creation.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.cs	New centralized CoreCLR registration entry point for all contract implementations/versions.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Microsoft.Diagnostics.DataContractReader.Contracts.csproj	Adds `InternalsVisibleTo` for the reader assembly (plus existing tests).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Thread_1.cs	Constructor now reads required globals internally (no factory-provided args).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/SyncBlock_1.cs	Constructor now reads required globals/type info internally.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs	Constructor now reads required globals and builds validations internally.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ReJIT_1.cs	Constructor now reads profiler control block internally.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/PlatformMetadata_1.cs	Constructor now reads platform metadata global internally.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.cs	Constructor now reads object-related globals/type offsets internally.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/PrecodeStubs_Common.cs	Precode stubs common base now pulls platform metadata + descriptor internally.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/PrecodeStubs_1.cs	Updated to use new `PrecodeStubsCommon(Target)` base ctor.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/PrecodeStubs_2.cs	Updated to use new `PrecodeStubsCommon(Target)` base ctor.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/PrecodeStubs_3.cs	Updated to use new `PrecodeStubsCommon(Target)` base ctor.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManager_1.cs	Constructor now reads range map global internally.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManager_2.cs	Constructor now reads range map global internally.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StressLog.cs	Removes factory; makes implementations accessible for registration.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ThreadFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/SyncBlockFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystemFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeInfoFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ReJITFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/PrecodeStubsFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/PlatformMetadataFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ObjectFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/NotificationsFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/LoaderFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/GCInfoFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GC/GCFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExceptionFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/EcmaMetadataFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/DebugInfo/DebugInfoFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/DebuggerFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/DacStreamsFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ConditionalWeakTableFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ComWrappersFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CodeVersionsFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/BuiltInCOMFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/AuxiliarySymbolsFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/StackWalkFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Signature/SignatureDecoderFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/SHashFactory.cs	Deleted (replaced by `CoreCLRContracts.Register`).
src/native/managed/cdac/tests/TestPlaceholderTarget.cs	Updates test registry to satisfy new abstract `Register` API.
src/native/managed/cdac/tests/DumpTests/DumpTestBase.cs	Uses `CoreCLRContracts.Register` when creating targets from dumps.
src/native/managed/cdac/tests/ContractDescriptor/ContractDescriptorBuilder.cs	Uses `CoreCLRContracts.Register` when creating descriptor targets in tests.
src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs	Ensures unmanaged entrypoints create targets with CoreCLR registrations.
src/native/managed/cdac/tests/TypeDescTests.cs	Replaces factories with direct contract impl construction in test setup.
src/native/managed/cdac/tests/ThreadTests.cs	Replaces factories with direct contract impl construction in test setup.
src/native/managed/cdac/tests/SyncBlockTests.cs	Replaces factories with direct contract impl construction in test setup.
src/native/managed/cdac/tests/RuntimeInfoTests.cs	Replaces RuntimeInfo factory usage with direct impl construction.
src/native/managed/cdac/tests/ReJITTests.cs	Replaces ReJIT factory usage with direct impl construction.
src/native/managed/cdac/tests/PrecodeStubsTests.cs	Replaces factory usage with version switch to select impl type in test setup.
src/native/managed/cdac/tests/ObjectTests.cs	Simplifies test setup by creating contracts directly (object/rts/syncblock).
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs	Replaces GC factory usage with direct impl construction.
src/native/managed/cdac/tests/MethodTableTests.cs	Replaces RTS factory usage with direct impl construction.
src/native/managed/cdac/tests/MethodDescTests.cs	Replaces RTS/Loader factories with direct impl construction.
src/native/managed/cdac/tests/LoaderTests.cs	Replaces Loader factory usage with direct impl construction.
src/native/managed/cdac/tests/GetRegisterNameTests.cs	Replaces RuntimeInfo factory usage with direct impl construction.
src/native/managed/cdac/tests/GCMemoryRegionTests.cs	Replaces GC factory usage with direct impl construction helper.
src/native/managed/cdac/tests/ExecutionManager/ExecutionManagerTests.cs	Replaces ExecutionManager factory with version switch in test setup.
src/native/managed/cdac/tests/DebuggerTests.cs	Replaces Debugger factory usage with direct impl construction.
src/native/managed/cdac/tests/DacStreamsTests.cs	Replaces DacStreams factory usage with direct impl construction.
src/native/managed/cdac/tests/CodeVersionsTests.cs	Replaces CodeVersions factory usage with direct impl construction.
src/native/managed/cdac/tests/BuiltInCOMTests.cs	Replaces BuiltInCOM factory usage with direct impl construction.
src/native/managed/cdac/tests/AuxiliarySymbolsTests.cs	Replaces AuxiliarySymbols factory usage with direct impl construction.

noahfalk · 2026-04-03T22:04:18Z

Overall this looks good. I was curious where the InternalsVisibleTo was needed and what it would take to avoid that.

Copilot

Pull request overview

Copilot reviewed 70 out of 70 changed files in this pull request and generated 4 comments.

Replace IContractFactory<T> and 26 named factory classes with a flat registration API: ContractRegistry.Register<T>(version, creator). CachingContractRegistry has zero contract knowledge — it's a pure (Type, int) -> creator registry. All contracts register from outside: - CoreCLRContracts.Register() — built-in CoreCLR contracts - External packages use the same API (e.g., NativeAOTContracts.Register()) ContractDescriptorTarget.TryCreate takes Action<ContractRegistry>[] for composable contract registration from multiple packages. Contract constructors simplified to take only Target — each reads its own globals internally (Object_1, GC_1, Thread_1, etc.). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…gFactory Replace direct StressLogFactory usage with CoreCLRContracts.Register and GetContract<IStressLog>(), removing the need for InternalsVisibleTo. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Add descriptive messages to NotImplementedException in GetContract - Remove unnecessary InternalsVisibleTo for Microsoft.Diagnostics.DataContractReader - Change TestContractRegistry.Register to throw NotSupportedException Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 68 out of 68 changed files in this pull request and generated 2 comments.

TestPlaceholderTarget.Builder now defaults to CoreCLRContracts.Register and supports AddContract<T>(version) for version-based resolution and AddMockContract<T>(mock) for injecting test doubles. TestContractRegistry supports Register, SetVersion, and SetMock with proper version lookup. Migrated 5 Builder-pattern test files from factory lambdas to version-based contract resolution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 68 out of 68 changed files in this pull request and generated 3 comments.

Convert all remaining cDAC test files to use the fluent Builder pattern instead of constructing TestPlaceholderTarget directly with manual SetContracts/SetupContractRegistry calls. Group 1 (SetupContractRegistry -> Builder): - CodeVersionsTests.cs: UseReader + AddContract/AddMockContract - DacStreamsTests.cs: MemoryBuilder + AddContract<IDacStreams> - ExecutionManagerTests.cs: UseReader + AddContract<IExecutionManager> - GCMemoryRegionTests.cs: UseReader + AddContract<IGC> Group 2 (Mock.Of<ContractRegistry> -> Builder): - SyncBlockTests.cs: AddContract<ISyncBlock> - LoaderTests.cs: 3 helpers converted to Builder - BuiltInCOMTests.cs: AddContract + AddMockContract<ISyncBlock> - RuntimeInfoTests.cs: AddContract<IRuntimeInfo> - MethodTableTests.cs: AddContract<IRuntimeTypeSystem> - MethodDescTests.cs: AddContract + AddMockContract - ObjectTests.cs: AddContract for default path, SetContracts for custom - RuntimeFunctionTests.cs: UseReader + AddMockContract - GetRegisterNameTests.cs: AddContract<IRuntimeInfo> - SOSDacInterface5Tests.cs: UseReader + AddMockContract - ClrDataExceptionStateTests.cs: 5 usages converted - ClrDataTaskTests.cs: MemoryBuilder for memory setup Also adds UseReader method to Builder for custom reader override, and removes unused Moq imports where no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 72 out of 72 changed files in this pull request and generated 2 comments.

max-charlamb · 2026-04-10T14:15:56Z

Overall this looks good. I was curious where the InternalsVisibleTo was needed and what it would take to avoid that.

This was from an earlier revision, I removed the InternalsVisibleTo attribute. Its no longer required.

Copilot AI review requested due to automatic review settings April 3, 2026 17:49

github-actions Bot added the area-Diagnostics-coreclr label Apr 3, 2026

dotnet-policy-service Bot assigned max-charlamb Apr 3, 2026

Copilot started reviewing on behalf of max-charlamb April 3, 2026 17:50 View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

max-charlamb force-pushed the dev/maxcharlamb/cdac-contract-registry-refactor branch from 1272071 to 723be81 Compare April 3, 2026 18:00

noahfalk reviewed Apr 3, 2026

View reviewed changes

Comment thread ...stics.DataContractReader.Contracts/Microsoft.Diagnostics.DataContractReader.Contracts.csproj Outdated

Copilot AI review requested due to automatic review settings April 8, 2026 22:01

Copilot started reviewing on behalf of max-charlamb April 8, 2026 22:03 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

max-charlamb and others added 3 commits April 9, 2026 13:28

max-charlamb force-pushed the dev/maxcharlamb/cdac-contract-registry-refactor branch from 2f0ff27 to 715c53c Compare April 9, 2026 17:31

Copilot AI review requested due to automatic review settings April 9, 2026 20:00

Copilot started reviewing on behalf of max-charlamb April 9, 2026 20:01 View session

max-charlamb commented Apr 9, 2026

View reviewed changes

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Comment thread src/native/managed/cdac/tests/ContractDescriptor/ContractDescriptorBuilder.cs

Comment thread src/native/managed/cdac/tests/TestPlaceholderTarget.cs

max-charlamb force-pushed the dev/maxcharlamb/cdac-contract-registry-refactor branch from 19efc80 to ef3659b Compare April 9, 2026 20:08

Copilot AI review requested due to automatic review settings April 9, 2026 20:10

max-charlamb force-pushed the dev/maxcharlamb/cdac-contract-registry-refactor branch from ef3659b to 007e5ff Compare April 9, 2026 20:10

Copilot started reviewing on behalf of max-charlamb April 9, 2026 20:11 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Comment thread src/native/managed/cdac/tests/TestPlaceholderTarget.cs

Comment thread src/native/managed/cdac/tests/TestPlaceholderTarget.cs

Comment thread src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs

Copilot AI review requested due to automatic review settings April 9, 2026 20:45

max-charlamb force-pushed the dev/maxcharlamb/cdac-contract-registry-refactor branch from 5d429fe to 7172588 Compare April 9, 2026 20:45

Copilot started reviewing on behalf of max-charlamb April 9, 2026 20:46 View session

max-charlamb force-pushed the dev/maxcharlamb/cdac-contract-registry-refactor branch from 7172588 to e5f5fde Compare April 9, 2026 20:54

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Comment thread src/native/managed/cdac/tests/PrecodeStubsTests.cs

Comment thread src/native/managed/cdac/tests/PrecodeStubsTests.cs

max-charlamb commented Apr 9, 2026

View reviewed changes

max-charlamb marked this pull request as ready for review April 10, 2026 14:14

max-charlamb requested a review from rcj1 April 10, 2026 14:15

max-charlamb requested a review from noahfalk April 10, 2026 14:16

rcj1 approved these changes Apr 10, 2026

View reviewed changes

max-charlamb merged commit ad46ca1 into dotnet:main Apr 10, 2026
56 of 59 checks passed

max-charlamb deleted the dev/maxcharlamb/cdac-contract-registry-refactor branch April 10, 2026 19:45

dotnet-maestro Bot mentioned this pull request Apr 11, 2026

[main] Source code updates from dotnet/runtime dotnet/dotnet#5987

Merged

Conversation

max-charlamb commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Changes

New registration API

CachingContractRegistry

Contract constructors

CoreCLRContracts.cs (new)

Deleted (26 files)

Enables

Testing

Uh oh!

dotnet-policy-service Bot commented Apr 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

noahfalk commented Apr 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

max-charlamb commented Apr 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

max-charlamb commented Apr 3, 2026 •

edited

Loading