Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional Validation for PauseThresholdWriter and ResumeThresholdWriter #77359

Merged
merged 6 commits into from
Nov 4, 2023
Merged

Additional Validation for PauseThresholdWriter and ResumeThresholdWriter #77359

merged 6 commits into from
Nov 4, 2023

Conversation

cdbullard
Copy link
Contributor

Fix #75166

Add in additional checks to ensure certain edge cases identified in the issue are handled appropriately to prevent programs from hanging indefinitely.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Oct 23, 2022
@halter73
Copy link
Member

halter73 commented Dec 6, 2022

I've also submitted a PR so the API docs include info about the PauseWriteThreshold of zero being unlimited on the property too. dotnet/dotnet-api-docs#8693

@halter73 halter73 requested a review from JamesNK December 6, 2022 22:34
Comment on lines 62 to 66

if (pauseWriterThreshold < 0)
{
ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.pauseWriterThreshold);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this block needs to move unless we're concerned about DefaultPauseWriterThreshold being less than 0. Where it was before would still throw an exception if pauseWriterThreshold was < -1 (due to the -1 check before it).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I think you want is the else if below these lines to just be if instead.

@adityamandaleeka
Copy link
Member

/azp run

@azure-pipelines
Copy link

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@AaronRobinsonMSFT
Copy link
Member

The assert is firing.

System.IO.Pipelines.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing  
  Discovering: System.IO.Pipelines.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.IO.Pipelines.Tests (found 408 of 413 test cases)
  Starting:    System.IO.Pipelines.Tests (parallel test collections = on, max threads = 2)
Process terminated. Assertion failed.
ResumeWriterThreshold is less than 1
   at System.IO.Pipelines.Pipe.AdvanceReader(BufferSegment consumedSegment, Int32 consumedIndex, BufferSegment examinedSegment, Int32 examinedIndex) in /_/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/Pipe.cs:line 503
   at System.IO.Pipelines.Pipe.AdvanceReader(SequencePosition& consumed, SequencePosition& examined) in /_/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/Pipe.cs:line 463
   at System.IO.Pipelines.Pipe.AdvanceReader(SequencePosition& consumed) in /_/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/Pipe.cs:line 450
   at System.IO.Pipelines.Pipe.DefaultPipeReader.AdvanceTo(SequencePosition consumed) in /_/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/Pipe.DefaultPipeReader.cs:line 28
   at System.IO.Pipelines.Tests.PipeWriterTests.Read() in /_/src/libraries/System.IO.Pipelines/tests/PipeWriterTests.cs:line 24
   at System.IO.Pipelines.Tests.PipeWriterTests.CanWriteIntoHeadlessBuffer() in /_/src/libraries/System.IO.Pipelines/tests/PipeWriterTests.cs:line 82
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args) in /_/src/coreclr/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.CoreCLR.cs:line 36
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr) in /_/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.cs:line 57
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture) in /_/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs:line 127
   at System.Reflection.MethodBase.Invoke(Object obj, Object[] parameters) in /_/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs:line 56
   at Xunit.Sdk.TestInvoker`1.CallTestMethod(Object testClassInstance) in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 150
   at Xunit.Sdk.TestInvoker`1.<>c__DisplayClass48_0.<<InvokeTestMethodAsync>b__1>d.MoveNext() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 257
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at Xunit.Sdk.TestInvoker`1.<>c__DisplayClass48_0.<InvokeTestMethodAsync>b__1()
   at Xunit.Sdk.ExecutionTimer.AggregateAsync(Func`1 asyncAction) in /_/src/xunit.execution/Sdk/Frameworks/ExecutionTimer.cs:line 48
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at Xunit.Sdk.ExecutionTimer.AggregateAsync(Func`1 asyncAction)
   at Xunit.Sdk.TestInvoker`1.<>c__DisplayClass48_0.<InvokeTestMethodAsync>b__0() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 242
   at Xunit.Sdk.ExceptionAggregator.RunAsync(Func`1 code) in /_/src/xunit.core/Sdk/ExceptionAggregator.cs:line 90
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at Xunit.Sdk.ExceptionAggregator.RunAsync(Func`1 code)
   at Xunit.Sdk.TestInvoker`1.InvokeTestMethodAsync(Object testClassInstance) in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 241
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at Xunit.Sdk.TestInvoker`1.InvokeTestMethodAsync(Object testClassInstance)
   at Xunit.Sdk.XunitTestInvoker.InvokeTestMethodAsync(Object testClassInstance) in /_/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestInvoker.cs:line 112
   at Xunit.Sdk.TestInvoker`1.<RunAsync>b__47_0() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 206
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at Xunit.Sdk.TestInvoker`1.<RunAsync>b__47_0()
   at Xunit.Sdk.ExceptionAggregator.RunAsync[T](Func`1 code) in /_/src/xunit.core/Sdk/ExceptionAggregator.cs:line 107
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at Xunit.Sdk.ExceptionAggregator.RunAsync[T](Func`1 code)
   at Xunit.Sdk.TestInvoker`1.RunAsync() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 189
   at Xunit.Sdk.XunitTestRunner.InvokeTestMethodAsync(ExceptionAggregator aggregator) in /_/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestRunner.cs:line 84
   at Xunit.Sdk.XunitTestRunner.InvokeTestAsync(ExceptionAggregator aggregator) in /_/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestRunner.cs:line 67
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at Xunit.Sdk.XunitTestRunner.InvokeTestAsync(ExceptionAggregator aggregator)
   at Xunit.Sdk.TestRunner`1.<>c__DisplayClass43_0.<RunAsync>b__0() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestRunner.cs:line 149
   at Xunit.Sdk.ExceptionAggregator.RunAsync[T](Func`1 code) in /_/src/xunit.core/Sdk/ExceptionAggregator.cs:line 107
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at Xunit.Sdk.ExceptionAggregator.RunAsync[T](Func`1 code)
   at Xunit.Sdk.TestRunner`1.RunAsync() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestRunner.cs:line 149
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at Xunit.Sdk.TestRunner`1.RunAsync()
   at Xunit.Sdk.XunitTestCaseRunner.RunTestAsync() in /_/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestCaseRunner.cs:line 139
   at Xunit.Sdk.TestCaseRunner`1.RunAsync() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestCaseRunner.cs:line 82
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at Xunit.Sdk.TestCaseRunner`1.RunAsync()
   at Xunit.Sdk.XunitTestCase.RunAsync(IMessageSink diagnosticMessageSink, IMessageBus messageBus, Object[] constructorArguments, ExceptionAggregator aggregator, CancellationTokenSource cancellationTokenSource) in /_/src/xunit.execution/Sdk/Frameworks/XunitTestCase.cs:line 162
   at Xunit.Sdk.XunitTestMethodRunner.RunTestCaseAsync(IXunitTestCase testCase) in /_/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestMethodRunner.cs:line 45
   at Xunit.Sdk.TestMethodRunner`1.RunTestCasesAsync() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestMethodRunner.cs:line 136
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at Xunit.Sdk.TestMethodRunner`1.RunTestCasesAsync()
   at Xunit.Sdk.TestMethodRunner`1.RunAsync() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestMethodRunner.cs:line 106
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at Xunit.Sdk.TestMethodRunner`1.RunAsync()
   at Xunit.Sdk.XunitTestClassRunner.RunTestMethodAsync(ITestMethod testMethod, IReflectionMethodInfo method, IEnumerable`1 testCases, Object[] constructorArguments) in /_/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestClassRunner.cs:line 168
   at Xunit.Sdk.TestClassRunner`1.RunTestMethodsAsync()
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at Xunit.Sdk.TestClassRunner`1.RunTestMethodsAsync()
   at Xunit.Sdk.TestClassRunner`1.RunAsync() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestClassRunner.cs:line 171
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at Xunit.Sdk.TestClassRunner`1.RunAsync()
   at Xunit.Sdk.XunitTestCollectionRunner.RunTestClassAsync(ITestClass testClass, IReflectionTypeInfo class, IEnumerable`1 testCases) in /_/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestCollectionRunner.cs:line 158
   at Xunit.Sdk.TestCollectionRunner`1.RunTestClassesAsync()
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at Xunit.Sdk.TestCollectionRunner`1.RunTestClassesAsync()
   at Xunit.Sdk.TestCollectionRunner`1.RunAsync() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestCollectionRunner.cs:line 101
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs:line 38
   at Xunit.Sdk.TestCollectionRunner`1.RunAsync()
   at Xunit.Sdk.XunitTestAssemblyRunner.RunTestCollectionAsync(IMessageBus messageBus, ITestCollection testCollection, IEnumerable`1 testCases, CancellationTokenSource cancellationTokenSource) in /_/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestAssemblyRunner.cs:line 235
   at Xunit.Sdk.XunitTestAssemblyRunner.<>c__DisplayClass14_2.<RunTestCollectionsAsync>b__2() in /_/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestAssemblyRunner.cs:line 184
   at System.Threading.Tasks.Task`1.InnerInvoke() in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Future.cs:line 501
   at System.Threading.Tasks.Task.<>c.<.cctor>b__281_0(Object obj) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs:line 2387
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/ExecutionContext.cs:line 179
   at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs:line 2345
   at System.Threading.Tasks.Task.ExecuteEntry() in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs:line 2258
   at System.Threading.Tasks.SynchronizationContextTaskScheduler.<>c.<.cctor>b__8_0(Object s) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TaskScheduler.cs:line 606
   at Xunit.Sdk.MaxConcurrencySyncContext.RunOnSyncContext(SendOrPostCallback callback, Object state) in /_/src/xunit.execution/Sdk/MaxConcurrencySyncContext.cs:line 106
   at Xunit.Sdk.MaxConcurrencySyncContext.<>c__DisplayClass11_0.<WorkerThreadProc>b__0(Object _) in /_/src/xunit.execution/Sdk/MaxConcurrencySyncContext.cs:line 96
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/ExecutionContext.cs:line 179
   at Xunit.Sdk.ExecutionContextHelper.Run(Object context, Action`1 action) in /_/src/xunit.execution/Sdk/Utility/ExecutionContextHelper.cs:line 110
   at Xunit.Sdk.MaxConcurrencySyncContext.WorkerThreadProc() in /_/src/xunit.execution/Sdk/MaxConcurrencySyncContext.cs:line 96
   at Xunit.Sdk.XunitWorkerThread.<>c.<QueueUserWorkItem>b__5_0(Object _) in /_/src/common/XunitWorkerThread.cs:line 37
   at System.Threading.Tasks.Task.InnerInvoke() in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs:line 2405
   at System.Threading.Tasks.Task.<>c.<.cctor>b__281_0(Object obj) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs:line 2387
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/ExecutionContext.cs:line 179
   at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs:line 2345
   at System.Threading.Tasks.Task.ExecuteEntryUnsafe(Thread threadPoolThread) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs:line 2283
   at System.Threading.Tasks.ThreadPoolTaskScheduler.<>c.<.cctor>b__10_0(Object s) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/ThreadPoolTaskScheduler.cs:line 35
   at System.Threading.Thread.StartCallback() in /_/src/coreclr/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs:line 104

Copy link
Member

@halter73 halter73 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like my comment from earlier was completely wrong. I must have gotten greater than and less than confused or something. I think @Turnerj is right about the else if.

@halter73 halter73 merged commit b9691b0 into dotnet:main Nov 4, 2023
109 checks passed
@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO.Pipelines community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PauseWriterThreshold and ResumeWriterThreshold options of PipeOptions not validated properly
7 participants