-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Handle concurrent dispose and WriteAsync calls #36988
Conversation
- lock the entire Write operation - Added a test that Completes both reader and writer in the middle of writing a large buffer
|
Still think TestHost.ResponseStream also needs fixing so you can't call write and complete at the same time on different threads. |
Yep. Looking at that. |
| { | ||
| for (int i = 0; i < 1000; i++) | ||
| { | ||
| var buffer = new byte[10000000]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Allocate this array outside of the try.
| { | ||
| // This is the multi segment copy | ||
| WriteMultiSegment(source.Span); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any concern of acquiring and releasing the lock immediately between here and FlushAsync? Can a call interleave in here that would affect the write call? I doubt it, but it's the only cause for concern I see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be fine. Efficiency.
- lock the entire Write operation - Added a test that Completes both reader and writer in the middle of writing a large buffer Commit migrated from dotnet/corefx@90d98b9
cc @benaadams
Found in ASP.NET Core test