Delete NetEventSource.Fail (#43579)
authorStephen Toub <stoub@microsoft.com>
Wed, 21 Oct 2020 17:33:59 +0000 (13:33 -0400)
committerGitHub <noreply@github.com>
Wed, 21 Oct 2020 17:33:59 +0000 (13:33 -0400)
At some point some Debug.Asserts/Fails were replaced by this NetEventSource.Fail helper, which both Debug.Fails and fires an EventSource event.  But asserts in our code base are intended for things that should never happen, and we needn't be emitting events for them (if we did want to emit events for them, we'd need to tackle the other ~20,000 Debug.Assert/Fails in the codebase.

I've deleted NetEventSource.Fail, and fixed up the call sites.  Some were simply replaced by Debug.Assert/Fail.  Some were deleted entirely, when from code inspection it looked like they could actually be hit, but were guarded by a check for the event source being enabled and thus were unlikely to have been triggered in our previous testing.  Etc.

28 files changed:
src/libraries/Common/src/Interop/Windows/SspiCli/SSPIAuthType.cs
src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs
src/libraries/Common/src/System/Net/ContextAwareResult.cs
src/libraries/Common/src/System/Net/Http/aspnetcore/NetEventSource.Common.cs
src/libraries/Common/src/System/Net/InternalException.cs
src/libraries/Common/src/System/Net/LazyAsyncResult.cs
src/libraries/Common/src/System/Net/Logging/NetEventSource.Common.cs
src/libraries/Common/src/System/Net/NTAuthentication.Common.cs
src/libraries/Common/src/System/Net/Security/NegotiateStreamPal.Unix.cs
src/libraries/Common/src/System/Net/Security/SSPIHandleCache.cs
src/libraries/Common/src/System/Net/Security/SecurityBuffer.Windows.cs
src/libraries/System.Net.Mail/src/System/Net/Mail/SmtpConnection.cs
src/libraries/System.Net.Primitives/tests/PalTests/Fakes/NetEventSource.cs
src/libraries/System.Net.Requests/src/System/Net/FtpControlStream.cs
src/libraries/System.Net.Requests/src/System/Net/FtpWebRequest.cs
src/libraries/System.Net.Requests/src/System/Net/TimerThread.cs
src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs
src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.cs
src/libraries/System.Net.Security/src/System/Net/Security/NegotiateStreamPal.Windows.cs
src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs
src/libraries/System.Net.Security/src/System/Net/Security/SslSessionsCache.cs
src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs
src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs
src/libraries/System.Net.Sockets/src/System/Net/Sockets/BaseOverlappedAsyncResult.Windows.cs
src/libraries/System.Net.Sockets/src/System/Net/Sockets/MultipleConnectAsync.cs
src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetEventSource.Sockets.cs
src/libraries/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.cs
src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs

index 415ef9d..4e6b841 100644 (file)
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 #nullable enable
+using System.Diagnostics;
 using System.Net.Security;
 using System.Runtime.InteropServices;
 
@@ -93,7 +94,7 @@ namespace System.Net
 
             if (status == 0 && qop == Interop.SspiCli.SECQOP_WRAP_NO_ENCRYPT)
             {
-                NetEventSource.Fail(this, $"Expected qop = 0, returned value = {qop}");
+                Debug.Fail($"Expected qop = 0, returned value = {qop}");
                 throw new InvalidOperationException(SR.net_auth_message_not_encrypted);
             }
 
index 57ce623..43d9d05 100644 (file)
@@ -226,6 +226,7 @@ namespace System.Net
 
         private static unsafe int EncryptDecryptHelper(OP op, ISSPIInterface secModule, SafeDeleteContext context, Span<SecurityBuffer> input, uint sequenceNumber)
         {
+            Debug.Assert(Enum.IsDefined<OP>(op), $"Unknown op: {op}");
             Debug.Assert(input.Length <= 3, "The below logic only works for 3 or fewer buffers.");
 
             Interop.SspiCli.SecBufferDesc sdcInOut = new Interop.SspiCli.SecBufferDesc(input.Length);
@@ -259,29 +260,13 @@ namespace System.Net
                 }
 
                 // The result is written in the input Buffer passed as type=BufferType.Data.
-                int errorCode;
-                switch (op)
+                int errorCode = op switch
                 {
-                    case OP.Encrypt:
-                        errorCode = secModule.EncryptMessage(context, ref sdcInOut, sequenceNumber);
-                        break;
-
-                    case OP.Decrypt:
-                        errorCode = secModule.DecryptMessage(context, ref sdcInOut, sequenceNumber);
-                        break;
-
-                    case OP.MakeSignature:
-                        errorCode = secModule.MakeSignature(context, ref sdcInOut, sequenceNumber);
-                        break;
-
-                    case OP.VerifySignature:
-                        errorCode = secModule.VerifySignature(context, ref sdcInOut, sequenceNumber);
-                        break;
-
-                    default:
-                        NetEventSource.Fail(null, $"Unknown OP: {op}");
-                        throw NotImplemented.ByDesignWithMessage(SR.net_MethodNotImplementedException);
-                }
+                    OP.Encrypt => secModule.EncryptMessage(context, ref sdcInOut, sequenceNumber),
+                    OP.Decrypt => secModule.DecryptMessage(context, ref sdcInOut, sequenceNumber),
+                    OP.MakeSignature => secModule.MakeSignature(context, ref sdcInOut, sequenceNumber),
+                    _ /* OP.VerifySignature */ => secModule.VerifySignature(context, ref sdcInOut, sequenceNumber),
+                };
 
                 // Marshalling back returned sizes / data.
                 for (int i = 0; i < input.Length; i++)
@@ -320,7 +305,7 @@ namespace System.Net
 
                         if (j >= input.Length)
                         {
-                            NetEventSource.Fail(null, "Output buffer out of range.");
+                            Debug.Fail("Output buffer out of range.");
                             iBuffer.size = 0;
                             iBuffer.offset = 0;
                             iBuffer.token = null;
@@ -328,27 +313,15 @@ namespace System.Net
                     }
 
                     // Backup validate the new sizes.
-                    if (iBuffer.offset < 0 || iBuffer.offset > (iBuffer.token == null ? 0 : iBuffer.token.Length))
-                    {
-                        NetEventSource.Fail(null, $"'offset' out of range.  [{iBuffer.offset}]");
-                    }
-
-                    if (iBuffer.size < 0 || iBuffer.size > (iBuffer.token == null ? 0 : iBuffer.token.Length - iBuffer.offset))
-                    {
-                        NetEventSource.Fail(null, $"'size' out of range.  [{iBuffer.size}]");
-                    }
+                    Debug.Assert(iBuffer.offset >= 0 && iBuffer.offset <= (iBuffer.token == null ? 0 : iBuffer.token.Length), $"'offset' out of range.  [{iBuffer.offset}]");
+                    Debug.Assert(iBuffer.size >= 0 && iBuffer.size <= (iBuffer.token == null ? 0 : iBuffer.token.Length - iBuffer.offset), $"'size' out of range.  [{iBuffer.size}]");
                 }
 
                 if (NetEventSource.Log.IsEnabled() && errorCode != 0)
                 {
-                    if (errorCode == Interop.SspiCli.SEC_I_RENEGOTIATE)
-                    {
-                        NetEventSource.Error(null, SR.Format(SR.event_OperationReturnedSomething, op, "SEC_I_RENEGOTIATE"));
-                    }
-                    else
-                    {
-                        NetEventSource.Error(null, SR.Format(SR.net_log_operation_failed_with_error, op, $"0x{0:X}"));
-                    }
+                    NetEventSource.Error(null, errorCode == Interop.SspiCli.SEC_I_RENEGOTIATE ?
+                        SR.Format(SR.event_OperationReturnedSomething, op, "SEC_I_RENEGOTIATE") :
+                        SR.Format(SR.net_log_operation_failed_with_error, op, $"0x{0:X}"));
                 }
 
                 return errorCode;
index 3e79411..6d3cd57 100644 (file)
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 #nullable enable
+using System.Diagnostics;
 using System.Threading;
 
 namespace System.Net
@@ -130,11 +131,7 @@ namespace System.Net
             {
                 if (InternalPeekCompleted)
                 {
-                    if ((_flags & StateFlags.ThreadSafeContextCopy) == 0)
-                    {
-                        NetEventSource.Fail(this, "Called on completed result.");
-                    }
-
+                    Debug.Assert((_flags & StateFlags.ThreadSafeContextCopy) != 0, "Called on completed result.");
                     throw new InvalidOperationException(SR.net_completed_result);
                 }
 
@@ -145,29 +142,19 @@ namespace System.Net
                 }
 
                 // Make sure the context was requested.
-                if (AsyncCallback == null && (_flags & StateFlags.CaptureContext) == 0)
-                {
-                    NetEventSource.Fail(this, "No context captured - specify a callback or forceCaptureContext.");
-                }
+                Debug.Assert(AsyncCallback != null || (_flags & StateFlags.CaptureContext) != 0, "No context captured - specify a callback or forceCaptureContext.");
 
                 // Just use the lock to block.  We might be on the thread that owns the lock which is great, it means we
                 // don't need a context anyway.
                 if ((_flags & StateFlags.PostBlockFinished) == 0)
                 {
-                    if (_lock == null)
-                    {
-                        NetEventSource.Fail(this, "Must lock (StartPostingAsyncOp()) { ... FinishPostingAsyncOp(); } when calling ContextCopy (unless it's only called after FinishPostingAsyncOp).");
-                    }
+                    Debug.Assert(_lock != null, "Must lock (StartPostingAsyncOp()) { ... FinishPostingAsyncOp(); } when calling ContextCopy (unless it's only called after FinishPostingAsyncOp).");
                     lock (_lock) { }
                 }
 
                 if (InternalPeekCompleted)
                 {
-                    if ((_flags & StateFlags.ThreadSafeContextCopy) == 0)
-                    {
-                        NetEventSource.Fail(this, "Result became completed during call.");
-                    }
-
+                    Debug.Assert((_flags & StateFlags.ThreadSafeContextCopy) != 0, "Result became completed during call.");
                     throw new InvalidOperationException(SR.net_completed_result);
                 }
 
@@ -197,11 +184,7 @@ namespace System.Net
         // object from being created.
         internal object? StartPostingAsyncOp(bool lockCapture)
         {
-            if (InternalPeekCompleted)
-            {
-                NetEventSource.Fail(this, "Called on completed result.");
-            }
-
+            Debug.Assert(!InternalPeekCompleted, "Called on completed result.");
             DebugProtectState(true);
 
             _lock = lockCapture ? new object() : null;
@@ -285,10 +268,7 @@ namespace System.Net
         // Returns whether the operation completed sync or not.
         private bool CaptureOrComplete(ref ExecutionContext? cachedContext, bool returnContext)
         {
-            if ((_flags & StateFlags.PostBlockStarted) == 0)
-            {
-                NetEventSource.Fail(this, "Called without calling StartPostingAsyncOp.");
-            }
+            Debug.Assert((_flags & StateFlags.PostBlockStarted) != 0, "Called without calling StartPostingAsyncOp.");
 
             // See if we're going to need to capture the context.
             bool capturingContext = AsyncCallback != null || (_flags & StateFlags.CaptureContext) != 0;
@@ -334,10 +314,7 @@ namespace System.Net
                 if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, "Skipping capture");
 
                 cachedContext = null;
-                if (AsyncCallback != null && !CompletedSynchronously)
-                {
-                    NetEventSource.Fail(this, "Didn't capture context, but didn't complete synchronously!");
-                }
+                Debug.Assert(AsyncCallback == null || CompletedSynchronously, "Didn't capture context, but didn't complete synchronously!");
             }
 
             // Now we want to see for sure what to do.  We might have just captured the context for no reason.
index 521db3c..174ad40 100644 (file)
@@ -35,9 +35,6 @@ namespace System.Net
     //       NetEventSource.Info(this, "literal string");  // arbitrary message with a literal string
     //   Debug.Asserts inside the logging methods will help to flag some misuse if the DEBUG_NETEVENTSOURCE_MISUSE compilation constant is defined.
     //   However, because it can be difficult by observation to understand all of the costs involved, guarding can be done everywhere.
-    // - NetEventSource.Fail calls typically do not need to be prefixed with an IsEnabled check, even if they allocate, as FailMessage
-    //   should only be used in cases similar to Debug.Fail, where they are not expected to happen in retail builds, and thus extra costs
-    //   don't matter.
     // - Messages can be strings, formattable strings, or any other object.  Objects (including those used in formattable strings) have special
     //   formatting applied, controlled by the Format method.  Partial specializations can also override this formatting by implementing a partial
     //   method that takes an object and optionally provides a string representation of it, in case a particular library wants to customize further.
@@ -70,7 +67,6 @@ namespace System.Net
         private const int AssociateEventId = 3;
         private const int InfoEventId = 4;
         private const int ErrorEventId = 5;
-        private const int CriticalFailureEventId = 6;
         private const int DumpArrayEventId = 7;
 
         // These events are implemented in NetEventSource.Security.cs.
@@ -253,40 +249,6 @@ namespace System.Net
             WriteEvent(ErrorEventId, thisOrContextObject, memberName ?? MissingMember, message);
         #endregion
 
-        #region Fail
-        /// <summary>Logs a fatal error and raises an assert.</summary>
-        /// <param name="thisOrContextObject">`this`, or another object that serves to provide context for the operation.</param>
-        /// <param name="formattableString">The message to be logged.</param>
-        /// <param name="memberName">The calling member.</param>
-        [NonEvent]
-        public static void Fail(object? thisOrContextObject, FormattableString formattableString, [CallerMemberName] string? memberName = null)
-        {
-            // Don't call DebugValidateArg on args, as we expect Fail to be used in assert/failure situations
-            // that should never happen in production, and thus we don't care about extra costs.
-
-            if (IsEnabled) Log.CriticalFailure(IdOf(thisOrContextObject), memberName, Format(formattableString));
-            Debug.Fail(Format(formattableString), $"{IdOf(thisOrContextObject)}.{memberName}");
-        }
-
-        /// <summary>Logs a fatal error and raises an assert.</summary>
-        /// <param name="thisOrContextObject">`this`, or another object that serves to provide context for the operation.</param>
-        /// <param name="message">The message to be logged.</param>
-        /// <param name="memberName">The calling member.</param>
-        [NonEvent]
-        public static void Fail(object? thisOrContextObject, object message, [CallerMemberName] string? memberName = null)
-        {
-            // Don't call DebugValidateArg on args, as we expect Fail to be used in assert/failure situations
-            // that should never happen in production, and thus we don't care about extra costs.
-
-            if (IsEnabled) Log.CriticalFailure(IdOf(thisOrContextObject), memberName, Format(message).ToString());
-            Debug.Fail(Format(message).ToString(), $"{IdOf(thisOrContextObject)}.{memberName}");
-        }
-
-        [Event(CriticalFailureEventId, Level = EventLevel.Critical, Keywords = Keywords.Debug)]
-        private void CriticalFailure(string thisOrContextObject, string? memberName, string? message) =>
-            WriteEvent(CriticalFailureEventId, thisOrContextObject, memberName ?? MissingMember, message);
-        #endregion
-
         #region DumpBuffer
         /// <summary>Logs the contents of a buffer.</summary>
         /// <param name="thisOrContextObject">`this`, or another object that serves to provide context for the operation.</param>
@@ -307,14 +269,8 @@ namespace System.Net
         [NonEvent]
         public static void DumpBuffer(object? thisOrContextObject, byte[] buffer, int offset, int count, [CallerMemberName] string? memberName = null)
         {
-            if (IsEnabled)
+            if (IsEnabled && offset >= 0 && offset <= buffer.Length - count)
             {
-                if (offset < 0 || offset > buffer.Length - count)
-                {
-                    Fail(thisOrContextObject, $"Invalid {nameof(DumpBuffer)} Args. Length={buffer.Length}, Offset={offset}, Count={count}", memberName);
-                    return;
-                }
-
                 count = Math.Min(count, MaxDumpSize);
 
                 byte[] slice = buffer;
index d1cbb26..1dd3293 100644 (file)
@@ -2,24 +2,20 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 #nullable enable
+using System.Diagnostics;
+
 namespace System.Net
 {
     internal sealed class InternalException : Exception
     {
         private readonly object? _unexpectedValue;
 
-        internal InternalException()
-        {
-            NetEventSource.Fail(this, "InternalException thrown.");
-        }
+        internal InternalException() : this(null) { }
 
-        internal InternalException(object unexpectedValue)
+        internal InternalException(object? unexpectedValue)
         {
+            Debug.Fail($"InternalException thrown for unexpected value: {unexpectedValue}");
             _unexpectedValue = unexpectedValue;
-            if (NetEventSource.Log.IsEnabled())
-            {
-                NetEventSource.Fail(this, $"InternalException thrown for unexpected value: {unexpectedValue}");
-            }
         }
 
         public override string Message => _unexpectedValue != null ?
index b31e995..fbdfd38 100644 (file)
@@ -257,15 +257,9 @@ namespace System.Net
                 // then the "result" parameter passed to InvokeCallback() will be ignored.
 
                 // It's an error to call after the result has been completed or with DBNull.
-                if (value == DBNull.Value)
-                {
-                    NetEventSource.Fail(this, "Result can't be set to DBNull - it's a special internal value.");
-                }
+                Debug.Assert(value != DBNull.Value, "Result can't be set to DBNull - it's a special internal value.");
 
-                if (InternalPeekCompleted)
-                {
-                    NetEventSource.Fail(this, "Called on completed result.");
-                }
+                Debug.Assert(!InternalPeekCompleted, "Called on completed result.");
                 _result = value;
             }
         }
index 3075eb1..fb10011 100644 (file)
@@ -35,9 +35,6 @@ namespace System.Net
     //       NetEventSource.Info(this, "literal string");  // arbitrary message with a literal string
     //   Debug.Asserts inside the logging methods will help to flag some misuse if the DEBUG_NETEVENTSOURCE_MISUSE compilation constant is defined.
     //   However, because it can be difficult by observation to understand all of the costs involved, guarding can be done everywhere.
-    // - NetEventSource.Fail calls typically do not need to be prefixed with an IsEnabled check, even if they allocate, as FailMessage
-    //   should only be used in cases similar to Debug.Fail, where they are not expected to happen in retail builds, and thus extra costs
-    //   don't matter.
     // - Messages can be strings, formattable strings, or any other object.  Objects (including those used in formattable strings) have special
     //   formatting applied, controlled by the Format method.  Partial specializations can also override this formatting by implementing a partial
     //   method that takes an object and optionally provides a string representation of it, in case a particular library wants to customize further.
@@ -74,7 +71,6 @@ namespace System.Net
         private const int AssociateEventId = 3;
         private const int InfoEventId = 4;
         private const int ErrorEventId = 5;
-        private const int CriticalFailureEventId = 6;
         private const int DumpArrayEventId = 7;
 
         // These events are implemented in NetEventSource.Security.cs.
@@ -153,46 +149,6 @@ namespace System.Net
             WriteEvent(ErrorEventId, thisOrContextObject, memberName ?? MissingMember, message);
         #endregion
 
-        #region Fail
-        /// <summary>Logs a fatal error and raises an assert.</summary>
-        /// <param name="thisOrContextObject">`this`, or another object that serves to provide context for the operation.</param>
-        /// <param name="formattableString">The message to be logged.</param>
-        /// <param name="memberName">The calling member.</param>
-        [NonEvent]
-#if NETCOREAPP
-        [DoesNotReturn]
-#endif
-        public static void Fail(object? thisOrContextObject, FormattableString formattableString, [CallerMemberName] string? memberName = null)
-        {
-            // Don't call DebugValidateArg on args, as we expect Fail to be used in assert/failure situations
-            // that should never happen in production, and thus we don't care about extra costs.
-
-            if (IsEnabled) Log.CriticalFailure(IdOf(thisOrContextObject), memberName, Format(formattableString));
-            Debug.Fail(Format(formattableString), $"{IdOf(thisOrContextObject)}.{memberName}");
-        }
-
-        /// <summary>Logs a fatal error and raises an assert.</summary>
-        /// <param name="thisOrContextObject">`this`, or another object that serves to provide context for the operation.</param>
-        /// <param name="message">The message to be logged.</param>
-        /// <param name="memberName">The calling member.</param>
-        [NonEvent]
-#if NETCOREAPP
-        [DoesNotReturn]
-#endif
-        public static void Fail(object? thisOrContextObject, object message, [CallerMemberName] string? memberName = null)
-        {
-            // Don't call DebugValidateArg on args, as we expect Fail to be used in assert/failure situations
-            // that should never happen in production, and thus we don't care about extra costs.
-
-            if (IsEnabled) Log.CriticalFailure(IdOf(thisOrContextObject), memberName, Format(message).ToString());
-            Debug.Fail(Format(message).ToString(), $"{IdOf(thisOrContextObject)}.{memberName}");
-        }
-
-        [Event(CriticalFailureEventId, Level = EventLevel.Critical, Keywords = Keywords.Debug)]
-        private void CriticalFailure(string thisOrContextObject, string? memberName, string? message) =>
-            WriteEvent(CriticalFailureEventId, thisOrContextObject, memberName ?? MissingMember, message);
-        #endregion
-
         #region Verbose
         /// <summary>Logs an info message at verbose mode.</summary>
         /// <param name="thisOrContextObject">`this`, or another object that serves to provide context for the operation.</param>
@@ -243,14 +199,8 @@ namespace System.Net
         [NonEvent]
         public static void DumpBuffer(object? thisOrContextObject, byte[] buffer, int offset, int count, [CallerMemberName] string? memberName = null)
         {
-            if (IsEnabled)
+            if (IsEnabled && offset >= 0 && offset <= buffer.Length - count)
             {
-                if (offset < 0 || offset > buffer.Length - count)
-                {
-                    Fail(thisOrContextObject, $"Invalid {nameof(DumpBuffer)} Args. Length={buffer.Length}, Offset={offset}, Count={count}", memberName);
-                    return;
-                }
-
                 count = Math.Min(count, MaxDumpSize);
 
                 byte[] slice = buffer;
index 5e58089..9c45708 100644 (file)
@@ -131,15 +131,8 @@ namespace System.Net
         internal SafeDeleteContext? GetContext(out SecurityStatusPal status)
         {
             status = new SecurityStatusPal(SecurityStatusPalErrorCode.OK);
-            if (!(IsCompleted && IsValidContext))
-            {
-                NetEventSource.Fail(this, "Should be called only when completed with success, currently is not!");
-            }
-
-            if (!IsServer)
-            {
-                NetEventSource.Fail(this, "The method must not be called by the client side!");
-            }
+            Debug.Assert(IsCompleted && IsValidContext, "Should be called only when completed with success, currently is not!");
+            Debug.Assert(IsServer, "The method must not be called by the client side!");
 
             if (!IsValidContext)
             {
@@ -306,10 +299,7 @@ namespace System.Net
 
         private string? GetClientSpecifiedSpn()
         {
-            if (!(IsValidContext && IsCompleted))
-            {
-                NetEventSource.Fail(this, "Trying to get the client SPN before handshaking is done!");
-            }
+            Debug.Assert(IsValidContext && IsCompleted, "Trying to get the client SPN before handshaking is done!");
 
             string? spn = NegotiateStreamPal.QueryContextClientSpecifiedSpn(_securityContext!);
 
index 7756478..2c32550 100644 (file)
@@ -585,13 +585,13 @@ namespace System.Net.Security
         {
             if (offset < 0 || offset > (buffer == null ? 0 : buffer.Length))
             {
-                NetEventSource.Fail(securityContext, "Argument 'offset' out of range");
+                Debug.Fail("Argument 'offset' out of range");
                 throw new ArgumentOutOfRangeException(nameof(offset));
             }
 
             if (count < 0 || count > (buffer == null ? 0 : buffer.Length - offset))
             {
-                NetEventSource.Fail(securityContext, "Argument 'count' out of range.");
+                Debug.Fail("Argument 'count' out of range.");
                 throw new ArgumentOutOfRangeException(nameof(count));
             }
 
@@ -603,13 +603,13 @@ namespace System.Net.Security
         {
             if (offset < 0 || offset > (buffer == null ? 0 : buffer.Length))
             {
-                NetEventSource.Fail(securityContext, "Argument 'offset' out of range");
+                Debug.Fail("Argument 'offset' out of range");
                 throw new ArgumentOutOfRangeException(nameof(offset));
             }
 
             if (count < 0 || count > (buffer == null ? 0 : buffer.Length - offset))
             {
-                NetEventSource.Fail(securityContext, "Argument 'count' out of range.");
+                Debug.Fail("Argument 'count' out of range.");
                 throw new ArgumentOutOfRangeException(nameof(count));
             }
 
index e6b7cbd..f08c43a 100644 (file)
@@ -21,25 +21,21 @@ namespace System.Net.Security
             try
             {
                 SafeCredentialReference? newRef = SafeCredentialReference.CreateReference(newHandle);
-
                 if (newRef == null)
                 {
                     return;
                 }
 
-                unchecked
-                {
-                    int index = Interlocked.Increment(ref s_current) & c_MaxCacheSize;
-                    newRef = Interlocked.Exchange<SafeCredentialReference>(ref s_cacheSlots[index], newRef);
-                }
+                int index = Interlocked.Increment(ref s_current) & c_MaxCacheSize;
+                newRef = Interlocked.Exchange<SafeCredentialReference>(ref s_cacheSlots[index], newRef);
 
                 newRef?.Dispose();
             }
             catch (Exception e)
             {
-                if (!ExceptionCheck.IsFatal(e))
+                if (NetEventSource.Log.IsEnabled() && !ExceptionCheck.IsFatal(e))
                 {
-                    if (NetEventSource.Log.IsEnabled()) NetEventSource.Fail(null, $"Attempted to throw: {e}");
+                    NetEventSource.Error(null, $"Attempted to throw: {e}");
                 }
             }
         }
index 2407616..b2a0bc8 100644 (file)
@@ -90,15 +90,8 @@ namespace System.Net.Security
 
         public SecurityBuffer(byte[]? data, int offset, int size, SecurityBufferType tokentype)
         {
-            if (offset < 0 || offset > (data == null ? 0 : data.Length))
-            {
-                NetEventSource.Fail(typeof(SecurityBuffer), $"'offset' out of range.  [{offset}]");
-            }
-
-            if (size < 0 || size > (data == null ? 0 : data.Length - offset))
-            {
-                NetEventSource.Fail(typeof(SecurityBuffer), $"'size' out of range.  [{size}]");
-            }
+            Debug.Assert(offset >= 0 && offset <= (data == null ? 0 : data.Length), $"'offset' out of range.  [{offset}]");
+            Debug.Assert(size >= 0 && size <= (data == null ? 0 : data.Length - offset), $"'size' out of range.  [{size}]");
 
             this.offset = data == null || offset < 0 ? 0 : Math.Min(offset, data.Length);
             this.size = data == null || size < 0 ? 0 : Math.Min(size, data.Length - this.offset);
@@ -118,10 +111,7 @@ namespace System.Net.Security
 
         public SecurityBuffer(int size, SecurityBufferType tokentype)
         {
-            if (size < 0)
-            {
-                NetEventSource.Fail(typeof(SecurityBuffer), $"'size' out of range.  [{size}]");
-            }
+            Debug.Assert(size >= 0, $"'size' out of range.  [{size}]");
 
             this.offset = 0;
             this.size = size;
index bd90d0c..1bebf22 100644 (file)
@@ -299,12 +299,8 @@ namespace System.Net.Mail
             if (ReferenceEquals(credential, CredentialCache.DefaultNetworkCredentials))
             {
 #if DEBUG
-                if (context != null && !context.IdentityRequested)
-                {
-                    NetEventSource.Fail(this, "Authentication required when it wasn't expected.  (Maybe Credentials was changed on another thread?)");
-                }
+                Debug.Assert(context == null || context.IdentityRequested, "Authentication required when it wasn't expected.  (Maybe Credentials was changed on another thread?)");
 #endif
-
                 try
                 {
                     ExecutionContext? x = context == null ? null : context.ContextCopy;
index fc92eae..157ff6a 100644 (file)
@@ -10,7 +10,6 @@ namespace System.Net
         public static NetEventSource Log = new NetEventSource();
 
         public static void Enter(object thisOrContextObject, object arg1 = null, object arg2 = null, object arg3 = null) { }
-        public static void Fail(object thisOrContextObject, object arg) { }
         public static void Info(object thisOrContextObject, object arg) { }
     }
 }
index 4fb8635..bd2bf87 100644 (file)
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Collections;
+using System.Diagnostics;
 using System.Globalization;
 using System.IO;
 using System.Net.Sockets;
@@ -665,10 +666,7 @@ namespace System.Net
 
             if (isPassive)
             {
-                if (port == -1)
-                {
-                    NetEventSource.Fail(this, "'port' not set.");
-                }
+                Debug.Assert(port != -1, "'port' not set.");
 
                 try
                 {
index 70de06c..b1f9f4a 100644 (file)
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
 using System.IO;
 using System.Net.Cache;
 using System.Net.Sockets;
@@ -1478,15 +1479,11 @@ namespace System.Net
 
                 if (stream != null)
                 {
-                    if (!(stream is ICloseEx))
-                    {
-                        NetEventSource.Fail(this, "The _stream member is not CloseEx hence the risk of connection been orphaned.");
-                    }
-
+                    Debug.Assert(stream is ICloseEx, "The _stream member is not CloseEx hence the risk of connection been orphaned.");
                     ((ICloseEx)stream).CloseEx(CloseExState.Abort | CloseExState.Silent);
                 }
-                if (connection != null)
-                    connection.Abort(ExceptionHelper.RequestAbortedException);
+
+                connection?.Abort(ExceptionHelper.RequestAbortedException);
             }
             catch (Exception exception)
             {
index c21afc9..a5778f5 100644 (file)
@@ -3,6 +3,7 @@
 
 using System.Collections;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Runtime.InteropServices;
 using System.Threading;
 
@@ -199,10 +200,7 @@ namespace System.Net
                 bool needProd = false;
                 lock (_timers)
                 {
-                    if (!(_timers.Prev!.Next == _timers))
-                    {
-                        NetEventSource.Fail(this, $"Tail corruption.");
-                    }
+                    Debug.Assert(_timers.Prev!.Next == _timers, $"Tail corruption.");
 
                     // If this is the first timer in the list, we need to create a queue handle and prod the timer thread.
                     if (_timers.Next == _timers)
index 557b2e2..093cd07 100644 (file)
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using Microsoft.Win32.SafeHandles;
+using System.Diagnostics;
 using System.Net.Security;
 using System.Runtime.InteropServices;
 using System.Security.Cryptography;
@@ -142,10 +143,7 @@ namespace System.Net
                         var elements = new Span<Interop.SspiCli.CERT_CHAIN_ELEMENT>((void*)sspiHandle!.DangerousGetHandle(), issuers.Length);
                         for (int i = 0; i < elements.Length; ++i)
                         {
-                            if (elements[i].cbSize <= 0)
-                            {
-                                NetEventSource.Fail(securityContext, $"Interop.SspiCli._CERT_CHAIN_ELEMENT size is not positive: {elements[i].cbSize}");
-                            }
+                            Debug.Assert(elements[i].cbSize > 0, $"Interop.SspiCli._CERT_CHAIN_ELEMENT size is not positive: {elements[i].cbSize}");
                             if (elements[i].cbSize > 0)
                             {
                                 byte[] x = new Span<byte>((byte*)elements[i].pCertContext, checked((int)elements[i].cbSize)).ToArray();
index 9d9ee92..aaabe82 100644 (file)
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
 using System.Security;
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
@@ -63,8 +64,7 @@ namespace System.Net
                         {
                             if (exception is CryptographicException || exception is SecurityException)
                             {
-                                NetEventSource.Fail(null,
-                                    $"Failed to open cert store, location: {storeLocation} exception: {exception}");
+                                Debug.Fail($"Failed to open cert store, location: {storeLocation} exception: {exception}");
                                 return null;
                             }
 
index e21530a..a2ea5bf 100644 (file)
@@ -95,19 +95,10 @@ namespace System.Net.Security
             bool success = SSPIWrapper.QueryBlittableContextAttributes(GlobalSSPI.SSPIAuth, securityContext, Interop.SspiCli.ContextAttribute.SECPKG_ATTR_SIZES, ref sizes);
             Debug.Assert(success);
 
-            try
+            int maxCount = checked(int.MaxValue - 4 - sizes.cbBlockSize - sizes.cbSecurityTrailer);
+            if (buffer.Length > maxCount)
             {
-                int maxCount = checked(int.MaxValue - 4 - sizes.cbBlockSize - sizes.cbSecurityTrailer);
-
-                if (buffer.Length > maxCount)
-                {
-                    throw new ArgumentOutOfRangeException(nameof(buffer.Length), SR.Format(SR.net_io_out_range, maxCount));
-                }
-            }
-            catch (Exception e) when (!ExceptionCheck.IsFatal(e))
-            {
-                NetEventSource.Fail(null, "Arguments out of range.");
-                throw;
+                throw new ArgumentOutOfRangeException(nameof(buffer.Length), SR.Format(SR.net_io_out_range, maxCount));
             }
 
             int resultSize = buffer.Length + sizes.cbSecurityTrailer + sizes.cbBlockSize;
@@ -187,13 +178,13 @@ namespace System.Net.Security
         {
             if (offset < 0 || offset > (buffer == null ? 0 : buffer.Length))
             {
-                NetEventSource.Fail(null, "Argument 'offset' out of range.");
+                Debug.Fail("Argument 'offset' out of range.");
                 throw new ArgumentOutOfRangeException(nameof(offset));
             }
 
             if (count < 0 || count > (buffer == null ? 0 : buffer.Length - offset))
             {
-                NetEventSource.Fail(null, "Argument 'count' out of range.");
+                Debug.Fail("Argument 'count' out of range.");
                 throw new ArgumentOutOfRangeException(nameof(count));
             }
 
@@ -249,7 +240,7 @@ namespace System.Net.Security
             // For the most part the arguments are verified in Decrypt().
             if (count < ntlmSignatureLength)
             {
-                NetEventSource.Fail(null, "Argument 'count' out of range.");
+                Debug.Fail("Argument 'count' out of range.");
                 throw new ArgumentOutOfRangeException(nameof(count));
             }
 
index 2261580..8e72784 100644 (file)
@@ -47,11 +47,7 @@ namespace System.Net.Security
             if (NetEventSource.Log.IsEnabled()) NetEventSource.Log.SecureChannelCtor(this, sslStream, sslAuthenticationOptions.TargetHost!, sslAuthenticationOptions.ClientCertificates, sslAuthenticationOptions.EncryptionPolicy);
 
             SslStreamPal.VerifyPackageInfo();
-
-            if (sslAuthenticationOptions.TargetHost == null)
-            {
-                NetEventSource.Fail(this, "sslAuthenticationOptions.TargetHost == null");
-            }
+            Debug.Assert(sslAuthenticationOptions.TargetHost != null, "sslAuthenticationOptions.TargetHost == null");
 
             _securityContext = null;
             _refreshCredentialNeeded = true;
@@ -550,13 +546,9 @@ namespace System.Net.Security
                 }
             }
 
-            if ((object?)clientCertificate != (object?)selectedCert && !clientCertificate!.Equals(selectedCert))
-            {
-                NetEventSource.Fail(this, "'selectedCert' does not match 'clientCertificate'.");
-            }
+            Debug.Assert((object?)clientCertificate == (object?)selectedCert || clientCertificate!.Equals(selectedCert), "'selectedCert' does not match 'clientCertificate'.");
 
-            if (NetEventSource.Log.IsEnabled())
-                NetEventSource.Info(this, $"Selected cert = {selectedCert}");
+            if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, $"Selected cert = {selectedCert}");
 
             try
             {
@@ -695,11 +687,7 @@ namespace System.Net.Security
                     throw new NotSupportedException(SR.net_ssl_io_no_server_cert);
                 }
 
-                if (!localCertificate.Equals(selectedCert))
-                {
-                    NetEventSource.Fail(this, "'selectedCert' does not match 'localCertificate'.");
-                }
-
+                Debug.Assert(localCertificate.Equals(selectedCert), "'selectedCert' does not match 'localCertificate'.");
                 _sslAuthenticationOptions.CertificateContext = SslStreamCertificateContext.Create(selectedCert);
             }
 
@@ -856,19 +844,10 @@ namespace System.Net.Security
 
             SslStreamPal.QueryContextStreamSizes(_securityContext!, out StreamSizes streamSizes);
 
-            try
-            {
-                _headerSize = streamSizes.Header;
-                _trailerSize = streamSizes.Trailer;
-                _maxDataSize = checked(streamSizes.MaximumMessage - (_headerSize + _trailerSize));
-
-                Debug.Assert(_maxDataSize > 0, "_maxDataSize > 0");
-            }
-            catch (Exception e) when (!ExceptionCheck.IsFatal(e))
-            {
-                NetEventSource.Fail(this, "StreamSizes out of range.");
-                throw;
-            }
+            _headerSize = streamSizes.Header;
+            _trailerSize = streamSizes.Trailer;
+            _maxDataSize = checked(streamSizes.MaximumMessage - (_headerSize + _trailerSize));
+            Debug.Assert(_maxDataSize > 0, "_maxDataSize > 0");
 
             SslStreamPal.QueryContextConnectionInfo(_securityContext!, out _connectionInfo);
         }
@@ -915,13 +894,13 @@ namespace System.Net.Security
         {
             if ((uint)offset > (uint)(payload == null ? 0 : payload.Length))
             {
-                NetEventSource.Fail(this, "Argument 'offset' out of range.");
+                Debug.Fail("Argument 'offset' out of range.");
                 throw new ArgumentOutOfRangeException(nameof(offset));
             }
 
             if ((uint)count > (uint)(payload == null ? 0 : payload.Length - offset))
             {
-                NetEventSource.Fail(this, "Argument 'count' out of range.");
+                Debug.Fail("Argument 'count' out of range.");
                 throw new ArgumentOutOfRangeException(nameof(count));
             }
 
index eccd25b..bb33143 100644 (file)
@@ -142,12 +142,9 @@ namespace System.Net.Security
         //
         internal static void CacheCredential(SafeFreeCredentials creds, byte[]? thumbPrint, SslProtocols sslProtocols, bool isServer, EncryptionPolicy encryptionPolicy)
         {
-            if (creds == null)
-            {
-                NetEventSource.Fail(null, "creds == null");
-            }
+            Debug.Assert(creds != null, "creds == null");
 
-            if (creds!.IsInvalid)
+            if (creds.IsInvalid)
             {
                 if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(null, $"Refused to cache an Invalid Handle {creds}, Current Cache Count = {s_cachedCreds.Count}");
                 return;
index 15cf9a4..b43c831 100644 (file)
@@ -885,10 +885,9 @@ namespace System.Net.Security
 
                         if (status.ErrorCode == SecurityStatusPalErrorCode.Renegotiate)
                         {
-                            // We determine above that we will not process it.
+                            // We determined above that we will not process it.
                             if (_handshakeWaiter == null)
                             {
-                                if (NetEventSource.Log.IsEnabled()) NetEventSource.Fail(this, "Renegotiation was requested but it is disallowed");
                                 throw new IOException(SR.net_ssl_io_renego);
                             }
 
@@ -1172,10 +1171,7 @@ namespace System.Net.Security
 
             int version = -1;
 
-            if (bytes.Length == 0)
-            {
-                NetEventSource.Fail(this, "Header buffer is not allocated.");
-            }
+            Debug.Assert(bytes.Length != 0, "Header buffer is not allocated.");
 
             // If the first byte is SSL3 HandShake, then check if we have a SSLv3 Type3 client hello.
             if (bytes[0] == (byte)TlsContentType.Handshake || bytes[0] == (byte)TlsContentType.AppData
index 25ddc61..89b474f 100644 (file)
@@ -250,16 +250,7 @@ namespace System.Net.Security
         public static unsafe SecurityStatusPal EncryptMessage(SafeDeleteSslContext securityContext, ReadOnlyMemory<byte> input, int headerSize, int trailerSize, ref byte[] output, out int resultSize)
         {
             // Ensure that there is sufficient space for the message output.
-            int bufferSizeNeeded;
-            try
-            {
-                bufferSizeNeeded = checked(input.Length + headerSize + trailerSize);
-            }
-            catch
-            {
-                NetEventSource.Fail(securityContext, "Arguments out of range");
-                throw;
-            }
+            int bufferSizeNeeded = checked(input.Length + headerSize + trailerSize);
             if (output == null || output.Length < bufferSizeNeeded)
             {
                 output = new byte[bufferSizeNeeded];
index d431c24..14e2b0e 100644 (file)
@@ -63,10 +63,7 @@ namespace System.Net.Sockets
             Debug.Assert(OperatingSystem.IsWindows());
             BaseOverlappedAsyncResult asyncResult = (BaseOverlappedAsyncResult)ThreadPoolBoundHandle.GetNativeOverlappedState(nativeOverlapped)!;
 
-            if (asyncResult.InternalPeekCompleted)
-            {
-                NetEventSource.Fail(null, $"asyncResult.IsCompleted: {asyncResult}");
-            }
+            Debug.Assert(!asyncResult.InternalPeekCompleted, $"asyncResult.IsCompleted: {asyncResult}");
             if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(null, $"errorCode:{errorCode} numBytes:{numBytes} nativeOverlapped:{(IntPtr)nativeOverlapped}");
 
             // Complete the IO and invoke the user's callback.
@@ -102,14 +99,11 @@ namespace System.Net.Sockets
                             out numBytes,
                             false,
                             out ignore);
+                        Debug.Assert(!success, $"Unexpectedly succeeded. errorCode:{errorCode} numBytes:{numBytes}");
                         if (!success)
                         {
                             socketError = SocketPal.GetLastSocketError();
                         }
-                        if (success)
-                        {
-                            NetEventSource.Fail(asyncResult, $"Unexpectedly succeeded. errorCode:{errorCode} numBytes:{numBytes}");
-                        }
                     }
                     catch (ObjectDisposedException)
                     {
index ff10fb2..17ca333 100644 (file)
@@ -41,12 +41,10 @@ namespace System.Net.Sockets
             Debug.Assert(!Monitor.IsEntered(_lockObject));
             lock (_lockObject)
             {
-                if (endPoint.AddressFamily != AddressFamily.Unspecified &&
-                    endPoint.AddressFamily != AddressFamily.InterNetwork &&
-                    endPoint.AddressFamily != AddressFamily.InterNetworkV6)
-                {
-                    NetEventSource.Fail(this, $"Unexpected endpoint address family: {endPoint.AddressFamily}");
-                }
+                Debug.Assert(endPoint.AddressFamily == AddressFamily.Unspecified ||
+                             endPoint.AddressFamily == AddressFamily.InterNetwork ||
+                             endPoint.AddressFamily == AddressFamily.InterNetworkV6,
+                             $"Unexpected endpoint address family: {endPoint.AddressFamily}");
 
                 _userArgs = args;
                 _endPoint = endPoint;
@@ -59,10 +57,7 @@ namespace System.Net.Sockets
                     return false;
                 }
 
-                if (_state != State.NotStarted)
-                {
-                    NetEventSource.Fail(this, "MultipleConnectAsync.StartConnectAsync(): Unexpected object state");
-                }
+                Debug.Assert(_state == State.NotStarted, "MultipleConnectAsync.StartConnectAsync(): Unexpected object state");
 
                 _state = State.DnsQuery;
 
@@ -106,18 +101,12 @@ namespace System.Net.Sockets
                     return true;
                 }
 
-                if (_state != State.DnsQuery)
-                {
-                    NetEventSource.Fail(this, "MultipleConnectAsync.DoDnsCallback(): Unexpected object state");
-                }
+                Debug.Assert(_state == State.DnsQuery, "MultipleConnectAsync.DoDnsCallback(): Unexpected object state");
 
                 try
                 {
                     _addressList = Dns.EndGetHostAddresses(result);
-                    if (_addressList == null)
-                    {
-                        NetEventSource.Fail(this, "MultipleConnectAsync.DoDnsCallback(): EndGetHostAddresses returned null!");
-                    }
+                    Debug.Assert(_addressList != null, "MultipleConnectAsync.DoDnsCallback(): EndGetHostAddresses returned null!");
                 }
                 catch (Exception e)
                 {
@@ -394,7 +383,7 @@ namespace System.Net.Sockets
                         break;
 
                     default:
-                        NetEventSource.Fail(this, "Unexpected object state");
+                        Debug.Fail($"Unexpected object state: {_state}");
                         break;
                 }
 
index 3c9be9d..27963c7 100644 (file)
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
 using System.Diagnostics.Tracing;
 using System.Net.Sockets;
 using System.Runtime.CompilerServices;
@@ -99,7 +100,7 @@ namespace System.Net
             {
                 if (offset < 0 || offset > buffer.Length - count)
                 {
-                    Fail(thisOrContextObject, $"Invalid {nameof(DumpBuffer)} Args. Length={buffer.Length}, Offset={offset}, Count={count}", memberName);
+                    Debug.Fail($"Invalid {nameof(DumpBuffer)} Args. Length={buffer.Length}, Offset={offset}, Count={count}");
                     return;
                 }
 
index 5293c47..9d6311e 100644 (file)
@@ -119,7 +119,7 @@ namespace System.Net.Sockets
             }
             catch (Exception exception) when (!ExceptionCheck.IsFatal(exception))
             {
-                NetEventSource.Fail(this, $"handle:{handle}, error:{exception}");
+                Debug.Fail($"handle:{handle}, error:{exception}");
                 throw;
             }
 #endif
@@ -150,11 +150,7 @@ namespace System.Net.Sockets
             }
             catch (Exception exception)
             {
-                if (!ExceptionCheck.IsFatal(exception))
-                {
-                    NetEventSource.Fail(this, $"handle:{handle}, error:{exception}");
-                }
-
+                Debug.Assert(ExceptionCheck.IsFatal(exception), $"handle:{handle}, error:{exception}");
                 ret = true;  // Avoid a second assert.
                 throw;
             }
@@ -162,10 +158,7 @@ namespace System.Net.Sockets
             {
                 _closeSocketThread = Environment.CurrentManagedThreadId;
                 _closeSocketTick = Environment.TickCount;
-                if (!ret)
-                {
-                    NetEventSource.Fail(this, $"ReleaseHandle failed. handle:{handle}");
-                }
+                Debug.Assert(ret, $"ReleaseHandle failed. handle:{handle}");
             }
 #endif
         }
index 2e8671d..dde4696 100644 (file)
@@ -3160,7 +3160,7 @@ namespace System.Net.Sockets
                     // That same map is implemented here just in case.
                     if (errorCode == SocketError.MessageSize)
                     {
-                        NetEventSource.Fail(this, "Returned WSAEMSGSIZE!");
+                        Debug.Fail("Returned WSAEMSGSIZE!");
                         errorCode = SocketError.IOPending;
                     }
                 }
@@ -4391,7 +4391,6 @@ namespace System.Net.Sockets
                     }
                     catch (ObjectDisposedException)
                     {
-                        NetEventSource.Fail(this, $"handle:{handle}, Closing the handle threw ObjectDisposedException.");
                     }
                 }
             }