From ed9b3576404c85749a0719f809dbf64b7440e365 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Mon, 10 Oct 2022 20:13:25 +0200 Subject: [PATCH] Require global opt-in for distributed transactions (#76376) Closes #76469 (cherry picked from commit 2070def2a1354c3c04f2ab0d39a5e68d9113d65b) --- .../ref/System.Transactions.Local.cs | 3 + .../ILLink.Suppressions.LibraryBuild.xml | 12 -- .../src/Resources/Strings.resx | 8 +- .../DtcProxyShim/DtcProxyShimFactory.cs | 68 ++++++++++-- .../System/Transactions/TransactionManager.cs | 62 +++++++++++ .../tests/OleTxTests.cs | 103 ++++++++++++++++-- 6 files changed, 224 insertions(+), 32 deletions(-) delete mode 100644 src/libraries/System.Transactions.Local/src/ILLink/ILLink.Suppressions.LibraryBuild.xml diff --git a/src/libraries/System.Transactions.Local/ref/System.Transactions.Local.cs b/src/libraries/System.Transactions.Local/ref/System.Transactions.Local.cs index 2f08b176bc27b..99e82f525ee68 100644 --- a/src/libraries/System.Transactions.Local/ref/System.Transactions.Local.cs +++ b/src/libraries/System.Transactions.Local/ref/System.Transactions.Local.cs @@ -4,6 +4,8 @@ // Changes to this file must follow the https://aka.ms/api-review process. // ------------------------------------------------------------------------------ +using System.Runtime.Versioning; + namespace System.Transactions { [System.Runtime.Versioning.UnsupportedOSPlatform("browser")] @@ -191,6 +193,7 @@ public static partial class TransactionManager [System.Diagnostics.CodeAnalysis.DisallowNullAttribute] public static System.Transactions.HostCurrentTransactionCallback? HostCurrentCallback { get { throw null; } set { } } public static System.TimeSpan MaximumTimeout { get { throw null; } set { } } + public static bool ImplicitDistributedTransactions { get; [System.Runtime.Versioning.SupportedOSPlatform("windows")] [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("Distributed transactions support may not be compatible with trimming. If your program creates a distributed transaction via System.Transactions, the correctness of the application cannot be guaranteed after trimming.")] set; } public static event System.Transactions.TransactionStartedEventHandler? DistributedTransactionStarted { add { } remove { } } public static void RecoveryComplete(System.Guid resourceManagerIdentifier) { } public static System.Transactions.Enlistment Reenlist(System.Guid resourceManagerIdentifier, byte[] recoveryInformation, System.Transactions.IEnlistmentNotification enlistmentNotification) { throw null; } diff --git a/src/libraries/System.Transactions.Local/src/ILLink/ILLink.Suppressions.LibraryBuild.xml b/src/libraries/System.Transactions.Local/src/ILLink/ILLink.Suppressions.LibraryBuild.xml deleted file mode 100644 index d002d008a2ac6..0000000000000 --- a/src/libraries/System.Transactions.Local/src/ILLink/ILLink.Suppressions.LibraryBuild.xml +++ /dev/null @@ -1,12 +0,0 @@ - - - - - ILLink - IL2026 - member - M:System.Transactions.DtcProxyShim.DtcProxyShimFactory.ConnectToProxyCore(System.String,System.Guid,System.Object,System.Boolean@,System.Byte[]@,System.Transactions.DtcProxyShim.ResourceManagerShim@) - This warning is left in the product so developers get an ILLink warning when trimming an app using this transaction support - - - diff --git a/src/libraries/System.Transactions.Local/src/Resources/Strings.resx b/src/libraries/System.Transactions.Local/src/Resources/Strings.resx index 7e75fee595f9a..647187affad81 100644 --- a/src/libraries/System.Transactions.Local/src/Resources/Strings.resx +++ b/src/libraries/System.Transactions.Local/src/Resources/Strings.resx @@ -423,4 +423,10 @@ Distributed transactions are currently unsupported in 32-bit processes. - + + Implicit distributed transactions have not been enabled. If you're intentionally starting a distributed transaction, set TransactionManager.ImplicitDistributedTransactions to true. + + + TransactionManager.ImplicitDistributedTransaction cannot be changed once set, or once System.Transactions distributed transactions have been initialized. Set this flag once at the start of your program. + + \ No newline at end of file diff --git a/src/libraries/System.Transactions.Local/src/System/Transactions/DtcProxyShim/DtcProxyShimFactory.cs b/src/libraries/System.Transactions.Local/src/System/Transactions/DtcProxyShim/DtcProxyShimFactory.cs index 35454d05adae4..2ec9b30f49812 100644 --- a/src/libraries/System.Transactions.Local/src/System/Transactions/DtcProxyShim/DtcProxyShimFactory.cs +++ b/src/libraries/System.Transactions.Local/src/System/Transactions/DtcProxyShim/DtcProxyShimFactory.cs @@ -21,6 +21,11 @@ internal sealed class DtcProxyShimFactory // at the same time. private static readonly object _proxyInitLock = new(); + // This object will perform the actual distributed transaction connection. + // It will be set only if TransactionManager.ImplicitDefaultTransactions + // is set to true, allowing the relevant code to be trimmed otherwise. + internal static ITransactionConnector? s_transactionConnector; + // Lock to protect access to listOfNotifications. private readonly object _notificationLock = new(); @@ -41,6 +46,7 @@ internal DtcProxyShimFactory(EventWaitHandle notificationEventHandle) // https://docs.microsoft.com/previous-versions/windows/desktop/ms678898(v=vs.85) [DllImport(Interop.Libraries.Xolehlp, CharSet = CharSet.Unicode, ExactSpelling = true, PreserveSig = false)] + [RequiresUnreferencedCode(TransactionManager.DistributedTransactionTrimmingWarning)] private static extern void DtcGetTransactionManagerExW( [MarshalAs(UnmanagedType.LPWStr)] string? pszHost, [MarshalAs(UnmanagedType.LPWStr)] string? pszTmName, @@ -49,7 +55,7 @@ private static extern void DtcGetTransactionManagerExW( object? pvConfigPararms, [MarshalAs(UnmanagedType.Interface)] out ITransactionDispenser ppvObject); - [RequiresUnreferencedCode("Distributed transactions support may not be compatible with trimming. If your program creates a distributed transaction via System.Transactions, the correctness of the application cannot be guaranteed after trimming.")] + [RequiresUnreferencedCode(TransactionManager.DistributedTransactionTrimmingWarning)] private static void DtcGetTransactionManager(string? nodeName, out ITransactionDispenser localDispenser) => DtcGetTransactionManagerExW(nodeName, null, Guids.IID_ITransactionDispenser_Guid, 0, null, out localDispenser); @@ -61,15 +67,27 @@ public void ConnectToProxy( out byte[] whereabouts, out ResourceManagerShim resourceManagerShim) { - switch (RuntimeInformation.ProcessArchitecture) + if (RuntimeInformation.ProcessArchitecture == Architecture.X86) + { + throw new PlatformNotSupportedException(SR.DistributedNotSupportedOn32Bits); + } + + lock (TransactionManager.s_implicitDistributedTransactionsLock) { - case Architecture.X86: - throw new PlatformNotSupportedException(SR.DistributedNotSupportedOn32Bits); + if (s_transactionConnector is null) + { + // We set TransactionManager.ImplicitDistributedTransactionsInternal, so that any attempt to change it + // later will cause an exception. + TransactionManager.s_implicitDistributedTransactions = false; + + throw new NotSupportedException(SR.ImplicitDistributedTransactionsDisabled); + } } - ConnectToProxyCore(nodeName, resourceManagerIdentifier, managedIdentifier, out nodeNameMatches, out whereabouts, out resourceManagerShim); + s_transactionConnector.ConnectToProxyCore(this, nodeName, resourceManagerIdentifier, managedIdentifier, out nodeNameMatches, out whereabouts, out resourceManagerShim); } + [RequiresUnreferencedCode(TransactionManager.DistributedTransactionTrimmingWarning)] private void ConnectToProxyCore( string? nodeName, Guid resourceManagerIdentifier, @@ -80,9 +98,7 @@ private void ConnectToProxyCore( { lock (_proxyInitLock) { -#pragma warning disable IL2026 // This warning is left in the product so developers get an ILLink warning when trimming an app using this transaction support DtcGetTransactionManager(nodeName, out ITransactionDispenser? localDispenser); -#pragma warning restore IL2026 // Check to make sure the node name matches. if (nodeName is not null) @@ -353,6 +369,8 @@ internal ITransactionTransmitter GetCachedTransmitter(ITransaction transaction) internal void ReturnCachedTransmitter(ITransactionTransmitter transmitter) { + // Note that due to race conditions, we may end up enqueuing above s_maxCachedInterfaces. + // This is benign, as this is only a best-effort cache, and there are no negative consequences. if (_cachedTransmitters.Count < s_maxCachedInterfaces) { transmitter.Reset(); @@ -375,10 +393,46 @@ internal ITransactionReceiver GetCachedReceiver() internal void ReturnCachedReceiver(ITransactionReceiver receiver) { + // Note that due to race conditions, we may end up enqueuing above s_maxCachedInterfaces. + // This is benign, as this is only a best-effort cache, and there are no negative consequences. if (_cachedReceivers.Count < s_maxCachedInterfaces) { receiver.Reset(); _cachedReceivers.Enqueue(receiver); } } + + internal interface ITransactionConnector + { + void ConnectToProxyCore( + DtcProxyShimFactory proxyShimFactory, + string? nodeName, + Guid resourceManagerIdentifier, + object managedIdentifier, + out bool nodeNameMatches, + out byte[] whereabouts, + out ResourceManagerShim resourceManagerShim); + } + + [RequiresUnreferencedCode(TransactionManager.DistributedTransactionTrimmingWarning)] + internal sealed class DtcTransactionConnector : ITransactionConnector + { + public void ConnectToProxyCore( + DtcProxyShimFactory proxyShimFactory, + string? nodeName, + Guid resourceManagerIdentifier, + object managedIdentifier, + out bool nodeNameMatches, + out byte[] whereabouts, + out ResourceManagerShim resourceManagerShim) + { + proxyShimFactory.ConnectToProxyCore( + nodeName, + resourceManagerIdentifier, + managedIdentifier, + out nodeNameMatches, + out whereabouts, + out resourceManagerShim); + } + } } diff --git a/src/libraries/System.Transactions.Local/src/System/Transactions/TransactionManager.cs b/src/libraries/System.Transactions.Local/src/System/Transactions/TransactionManager.cs index 7ce21c51bad23..93a6fefadb88b 100644 --- a/src/libraries/System.Transactions.Local/src/System/Transactions/TransactionManager.cs +++ b/src/libraries/System.Transactions.Local/src/System/Transactions/TransactionManager.cs @@ -4,8 +4,12 @@ using System.Collections; using System.Diagnostics.CodeAnalysis; using System.IO; +using System.Runtime.Versioning; using System.Threading; using System.Transactions.Configuration; +#if WINDOWS +using System.Transactions.DtcProxyShim; +#endif using System.Transactions.Oletx; namespace System.Transactions @@ -29,6 +33,10 @@ public static class TransactionManager private static TransactionTable? s_transactionTable; private static TransactionStartedEventHandler? s_distributedTransactionStartedDelegate; + + internal const string DistributedTransactionTrimmingWarning = + "Distributed transactions support may not be compatible with trimming. If your program creates a distributed transaction via System.Transactions, the correctness of the application cannot be guaranteed after trimming."; + public static event TransactionStartedEventHandler? DistributedTransactionStarted { add @@ -391,6 +399,60 @@ public static TimeSpan MaximumTimeout } } + /// + /// Controls whether usage of System.Transactions APIs that require escalation to a distributed transaction will do so; + /// if your application requires distributed transaction, opt into using them by setting this to . + /// If set to (the default), escalation to a distributed transaction will throw a . + /// +#if WINDOWS + public static bool ImplicitDistributedTransactions + { + get => DtcProxyShimFactory.s_transactionConnector is not null; + + [SupportedOSPlatform("windows")] + [RequiresUnreferencedCode(DistributedTransactionTrimmingWarning)] + set + { + lock (s_implicitDistributedTransactionsLock) + { + // Make sure this flag can only be set once, and that once distributed transactions have been initialized, + // it's frozen. + if (s_implicitDistributedTransactions is null) + { + s_implicitDistributedTransactions = value; + + if (value) + { + DtcProxyShimFactory.s_transactionConnector ??= new DtcProxyShimFactory.DtcTransactionConnector(); + } + } + else if (value != s_implicitDistributedTransactions) + { + throw new InvalidOperationException(SR.ImplicitDistributedTransactionsCannotBeChanged); + } + } + } + } + + internal static bool? s_implicitDistributedTransactions; + internal static object s_implicitDistributedTransactionsLock = new(); +#else + public static bool ImplicitDistributedTransactions + { + get => false; + + [SupportedOSPlatform("windows")] + [RequiresUnreferencedCode(DistributedTransactionTrimmingWarning)] + set + { + if (value) + { + throw new PlatformNotSupportedException(SR.DistributedNotSupported); + } + } + } +#endif + // This routine writes the "header" for the recovery information, based on the // type of the calling object and its provided parameter collection. This information // we be read back by the static Reenlist method to create the necessary transaction diff --git a/src/libraries/System.Transactions.Local/tests/OleTxTests.cs b/src/libraries/System.Transactions.Local/tests/OleTxTests.cs index 739ca93afeb2d..0d52469c696d8 100644 --- a/src/libraries/System.Transactions.Local/tests/OleTxTests.cs +++ b/src/libraries/System.Transactions.Local/tests/OleTxTests.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.IO; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Threading; using Microsoft.DotNet.RemoteExecutor; @@ -492,6 +493,80 @@ public void GetDtcTransaction() Retry(() => Assert.Equal(TransactionStatus.Committed, tx.TransactionInformation.Status)); }); + [ConditionalFact(nameof(IsRemoteExecutorSupportedAndNotNano))] + public void Distributed_transactions_require_ImplicitDistributedTransactions_true() + { + // Temporarily skip on 32-bit where we have an issue. + if (!Environment.Is64BitProcess) + { + return; + } + + using var _ = RemoteExecutor.Invoke(() => + { + Assert.False(TransactionManager.ImplicitDistributedTransactions); + + using var tx = new CommittableTransaction(); + + Assert.Throws(MinimalOleTxScenario); + }); + } + + [ConditionalFact(nameof(IsRemoteExecutorSupportedAndNotNano))] + public void ImplicitDistributedTransactions_cannot_be_changed_after_being_set() + { + // Temporarily skip on 32-bit where we have an issue. + if (!Environment.Is64BitProcess) + { + return; + } + + using var _ = RemoteExecutor.Invoke(() => + { + TransactionManager.ImplicitDistributedTransactions = true; + + Assert.Throws(() => TransactionManager.ImplicitDistributedTransactions = false); + }); + } + + [ConditionalFact(nameof(IsRemoteExecutorSupportedAndNotNano))] + public void ImplicitDistributedTransactions_cannot_be_changed_after_being_read_as_true() + { + // Temporarily skip on 32-bit where we have an issue. + if (!Environment.Is64BitProcess) + { + return; + } + + using var _ = RemoteExecutor.Invoke(() => + { + TransactionManager.ImplicitDistributedTransactions = true; + + MinimalOleTxScenario(); + + Assert.Throws(() => TransactionManager.ImplicitDistributedTransactions = false); + TransactionManager.ImplicitDistributedTransactions = true; + }); + } + + [ConditionalFact(nameof(IsRemoteExecutorSupportedAndNotNano))] + public void ImplicitDistributedTransactions_cannot_be_changed_after_being_read_as_false() + { + // Temporarily skip on 32-bit where we have an issue. + if (!Environment.Is64BitProcess) + { + return; + } + + using var _ = RemoteExecutor.Invoke(() => + { + Assert.Throws(MinimalOleTxScenario); + + Assert.Throws(() => TransactionManager.ImplicitDistributedTransactions = true); + TransactionManager.ImplicitDistributedTransactions = false; + }); + } + private static void Test(Action action) { // Temporarily skip on 32-bit where we have an issue. @@ -500,6 +575,8 @@ private static void Test(Action action) return; } + TransactionManager.ImplicitDistributedTransactions = true; + // In CI, we sometimes get XACT_E_TMNOTAVAILABLE; when it happens, it's typically on the very first // attempt to connect to MSDTC (flaky/slow on-demand startup of MSDTC), though not only. // This catches that error and retries. @@ -549,23 +626,25 @@ private static void Retry(Action action) } } + static void MinimalOleTxScenario() + { + using var tx = new CommittableTransaction(); + + var enlistment1 = new TestEnlistment(Phase1Vote.Prepared, EnlistmentOutcome.Committed); + var enlistment2 = new TestEnlistment(Phase1Vote.Prepared, EnlistmentOutcome.Committed); + + tx.EnlistDurable(Guid.NewGuid(), enlistment1, EnlistmentOptions.None); + tx.EnlistDurable(Guid.NewGuid(), enlistment2, EnlistmentOptions.None); + + tx.Commit(); + } + public class OleTxFixture { // In CI, we sometimes get XACT_E_TMNOTAVAILABLE on the very first attempt to connect to MSDTC; // this is likely due to on-demand slow startup of MSDTC. Perform pre-test connecting with retry // to ensure that MSDTC is properly up when the first test runs. public OleTxFixture() - => Test(() => - { - using var tx = new CommittableTransaction(); - - var enlistment1 = new TestEnlistment(Phase1Vote.Prepared, EnlistmentOutcome.Committed); - var enlistment2 = new TestEnlistment(Phase1Vote.Prepared, EnlistmentOutcome.Committed); - - tx.EnlistDurable(Guid.NewGuid(), enlistment1, EnlistmentOptions.None); - tx.EnlistDurable(Guid.NewGuid(), enlistment2, EnlistmentOptions.None); - - tx.Commit(); - }); + => Test(MinimalOleTxScenario); } }