Skip to content

Fix | Load and resolve retry logic providers into the current AssemblyLoadContext - #4588

Open
edwardneal wants to merge 2 commits into
dotnet:mainfrom
edwardneal:feat/retrylogic-current-alc
Open

Fix | Load and resolve retry logic providers into the current AssemblyLoadContext#4588
edwardneal wants to merge 2 commits into
dotnet:mainfrom
edwardneal:feat/retrylogic-current-alc

Conversation

@edwardneal

Copy link
Copy Markdown
Contributor

Description

This PR addresses two interrelated bugs, both of which are triggered when SqlClient is loaded into an alternate AssemblyLoadContext. Both of these relate to SqlConfigurableRetryLogicLoader.

The first bug is that when using the managed SNI, SqlClient blocks an unloadable AssemblyLoadContext from being unloaded. This is completely due to the event handlers: it adds a handler to the default ALC's Resolving event, and this strong reference from the default ALC to a type loaded into a secondary ALC prevents that secondary ALC from being unloaded. This is fixed by making sure that the currently-executing ALC unhooks the Resolving handler when it unloads.

The second, related, bug is that we load a configuration's RetryLogicType into the default AssemblyLoadContext. This means that the type will never be unloaded with the secondary AssemblyLoadContext - and because it needs to reference a type in that secondary ALC, the secondary ALC won't unload either. Fixing this means that we need to load RetryLogicType into the current AssemblyLoadContext, not the default one.

To avoid a regression in fixing the second bug, we must also hook to the current AssemblyLoadContext's Resolving handler. This is called to resolve transitive dependencies of the assembly which has to be loaded to locate a configuration's RetryLogicType. The nuance here is that only the current AssemblyLoadContext's Resolving handler is called - not the default handler. If those two differ, we'll find ourselves in a position where a transitive dependency can fail to load (or worse, the wrong dependency may load - different ALCs may have different base paths.)

With these bugs resolved, #1687 is fixed when using the managed SNI.

I don't think that SqlClient can implement a fix to make the native SNI compatible with unloadable AssemblyLoadContexts - there's a rooted SNILoadHandle which calls SniInitialize and SniTerminate. Unless these two methods are signalling and waiting a semaphore in native code, they seem to perform process-wide initialization and teardown. I think having an AssemblyLoadContext remain uncollectible is safer behaviour than having multiple ALCs referring to the process-wide SNI; one might have an in-flight async request, while the other might be unloading and performing SNI teardown.

Issues

Contributes to #1687.

Testing

Existing automated tests (SqlConfigurationManagerReliabilityTest) continue to pass. I've also added a new test, UnloadableAssemblyLoadContextTest. This test covers the scenario where users are loading and unloading AssemblyLoadContexts.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

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

Labels

None yet

Projects

Status: To triage

Development

Successfully merging this pull request may close these issues.

1 participant