dotnet / dotnet/SqlClient

Redesign configurable retry logic assembly loading (dedicated ALC and/or explicit registration API)

Open
#4,623 0 comments 1 reaction 1 assignee Claimed by @priyankatiwari08 View on GitHub
Area\Engineering
Dominant language
C#
Stars
989
Forks
340
Avg merge
4d 18h
Merged PRs (30d)
69

Description

## Background

`SqlConfigurableRetryLogicLoader` loads a user-supplied retry logic type named by the `retryLogicType` configuration string. To do that it resolves a `Type` by name, probes a directory on disk for the assembly, and subscribes a handler to `AssemblyLoadContext.Default.Resolving` so the assembly's private dependencies can be found.

#4547 mitigates the security problem with that design (MSRC-tracked): it probes `AppContext.BaseDirectory` rather than the working directory, and subscribes the resolving handler only while a configured type is being resolved and constructed, rather than for the lifetime of the process.

That PR is deliberately scoped to what is safe to backport to 7.0 and 6.1. It does not change the underlying design, and two larger questions raised during its review were deferred here.

## 1. Load configured retry logic into a dedicated `AssemblyLoadContext`

Raised by @paulmedynski in [review of #4547](https://github.com/dotnet/SqlClient/pull/4547#discussion_r3852163994).

Today the plugin assembly and its dependencies are loaded into the default ALC, which is why a `Default.Resolving` handler is needed at all. A dedicated ALC with an `AssemblyDependencyResolver` would let the plugin's dependencies resolve without touching global assembly resolution, and would close a residual gap that scoping alone cannot: a provider that loads a private dependency *after* it has been constructed is still unresolvable under #4547, which is why that PR ships `Switch.Microsoft.Data.SqlClient.UseLegacyRetryLogicAssemblyResolution` as an escape hatch.

Design considerations:

- **Type identity.** The plugin must bind to the *same* `Microsoft.Data.SqlClient` the host is using. If a dedicated ALC loads a second copy, every cast to `SqlRetryLogicBaseProvider` throws `InvalidCastException`. The `Load` override needs to defer to the parent context for SqlClient itself.
- There is a related latent bug in `main` independent of this work: `AssemblyResolver` calls `AssemblyLoadContext.Default.LoadFromAssemblyPath`, so if SqlClient is itself hosted in a secondary ALC the plugin lands in the wrong context. #4588 addresses this.
- Interaction with #1687 (ALC unloadability) should be considered together.

## 2. Explicit registration API instead of loading application assemblies

Raised by @paulmedynski in [the same review](https://github.com/dotnet/SqlClient/pull/4547#discussion_r3852188606) — paraphrasing: it isn't clear why SqlClient should be loading application assemblies at all, when the application could hand us the provider directly.

An API such as registering a `SqlRetryLogicBaseProvider` instance or factory in code removes the need to resolve types by name, probe directories, or hook assembly resolution. It is also trimming- and AOT-friendly, which the reflection-based path is not — @edwardneal separately noted that the `System.Configuration` dependency in this path is trim/AOT-incompatible.

Open questions:

- Whether the config-file path is deprecated, kept alongside the API, or kept only for .NET Framework.
- Whether registration is global, per-`SqlConnection`, or both.
- Migration guidance for existing `retryLogicType` consumers.

## Scope

Targeting 8.0. @cheenamalhotra noted the 8.0 API direction may need rethinking, to be discussed separately from the hotfix-branch mitigation.

## References

- #4547 — the mitigation being backported to 7.1.0 / 7.0 / 6.1
- #4588 — loads retry logic into the current ALC; overlaps this area
- #1687 — SqlClient is not compatible with `AssemblyLoadContext` unloading
- #2214, #2134 — earlier reports against this loader

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.