Repository navigation
Proposal: generate sync attestation methods from the async ones with a source generator #4799
Description
Activity
🔍 Triage Summary
Check Result Issue type Feature Environment All required environment details provided for investigation (not a bug report; environment validation not applicable) Area Area\Engineering(build tooling / new build-time dependency; touches enclave providers)Duplicates None found (#1947 is referenced for the trim-analyzer errors) Regression Not indicated Analysis
The proposal adds a third-party build-time source generator (Zomp.SyncMethodGenerator) to derive sync enclave-provider methods from their async twins. This would fix the drift between
MakeRequestandMakeRequestAsyncinHostGuardianServiceEnclaveProvider(source-generated vs reflection JSON), which causes the trim errors in #1947. The main concerns are supply-chain approval for a new package on the governed feed, maintainer-owned dependency disclosure, and behavioral risk from sync-over-async generation. Severity is P3 (maintainability / trimming).Next Steps
- Maintainers need to decide whether adding
Zomp.SyncMethodGeneratorto the governed NuGet feed is acceptable (security/compliance review, license, andDirectory.Packages.props/3rd-party-package-versionspolicy). - Author: please don't open a PR until that is approved. Once it is, open it without the temporary nuget.org mapping commit and with unit tests covering both the sync and async paths, on net462 and net8.0 and net9.0.
- Consider whether the narrower fix (making
MakeRequestAsyncuse the source-generated JSON context) resolves NativeAOT support: get to zero warnings #1947 without a new dependency.
Note: This triage summary is auto-generated by an AI agent. The analysis and suggestions above have not been verified by a human maintainer. Please treat as preliminary guidance only.
Generated by SqlClient Issue Auto-Triage for #4799 · copilot · auto · 23.8 AIC · ⌖ 11.7 AIC · ⊞ 13.4K · ◷
- Maintainers need to decide whether adding
The generated code is synchronous, not sync-over-async:
Task.DelaybecomesThread.Sleep,DeserializeAsyncbecomesDeserialize, and so on. The one blocking call in the trial isHttpClient.GetStreamAsync, which the hand-writtenMakeRequestalready blocks on becauseHttpClienthas no synchronousGetStream.Agreed that using the source-generated context in
MakeRequestAsyncalone fixes the trim errors. The broader case is drift: the async VBS path also skips the enclave key binding check the sync path does (#4798).For comparison, the dependency-free alternative is the
bool asyncpattern the BCL networking stack uses: one method withif (async) await x.ReadAsync(...) else x.Read(...)at each I/O call. It needs no package approval and prevents drift the same way. The generator keeps the async method free of those branches, which matters more as more pairs are converted. For the three methods here, either would work.I've also updated the trial so the sync path uses
HttpClient.Sendon .NET, which removes its sync-over-async call there; .NET Framework still blocks, as today.
The enclave providers keep hand-written sync and async twins of the same methods, and they have drifted. In
HostGuardianServiceEnclaveProvider,MakeRequestdeserializes through the source-generated JSON context whileMakeRequestAsyncuses reflection, which produces two trim-analyzer errors (#1947).Proposal: write the async method only and generate the sync one with Zomp.SyncMethodGenerator, a build-time source generator (
PrivateAssets="all", no runtime or package dependency). Full disclosure: I maintain it.A trial on the VBS/HGS provider converts
MakeRequest,GetSigningCertificateandVerifyAttestationInfo: -106/+55 lines, the two trim errors gone, unit tests passing on net462 and net8.0. Branch: virzak/SqlClient@main...dev/automation/sync-method-generatorThe blocker is the governed feed:
Zomp.SyncMethodGeneratorisn't on it, so the trial temporarily maps it to nuget.org. Would you approve adding it? If so, I'll open a PR without that commit.