Porting async implementation and adding TPL overrides for SslStream Read/WriteAsync#5541
Conversation
|
@benaadams If you have some time, it would be interesting to check if this fixes your issue while we wait for the PR review. |
|
@CIPop can I use as is (e.g. I don't need the other |
@benaadams If your code is using a true async stream (e.g. NetworkStream), I don't think you need the Stream enhancements for this to unblock you. |
|
@CIPop its pure async; with the sync being simulated - which is why it interacted quite badly with the previous |
There was a problem hiding this comment.
We should change the port from a fixed '4567' to a dynamic port, i.e. '0'. Otherwise, if a single CI machine is running multiple instances of PR validation, this port might be busy running another test.
There was a problem hiding this comment.
Good point. I've missed that when I was adapting the test for CI.
|
@CIPop I'm having trouble building it - though I think its a general corefx thing, so will start from scratch. As an aside the CI tests can't compile e.g. |
Thanks @benaadams. Addressed that and will update the PR shortly. (The UT Fakes needed updated.) |
|
@CIPop I can't build, I think I might be between tools; though it starts with this It does build some stuff though. If you could put the dll and pdb somewhere I could give it a go. |
There was a problem hiding this comment.
Nit: These could all be readonly. They should also have an s_ prefix.
|
LGTM |
|
The two Windows_NT failures are: |
Since they are intermittent, I believe these are #4467. I found a few issues with both the test patterns and the Mock infra. |
@benaadams I don't see these errors but I think you're using PoshGit. Try to open a |
…ppers and a simple test proposed by the community.
|
LGTM |
Porting async implementation and adding TPL overrides for SslStream Read/WriteAsync
|
Thanks for the review! |
|
@CIPop still having a lock up post this fix; debugging into it (need to determine cause using v4.0.0-rc2-23722) |
|
@CIPop still seem to be dropping into a |
|
@benaadams This fix is only for corefx. net451 would use .Net Desktop implementations. Can you repro this on CoreFX? |
|
Trying |
|
Yes; on The current clr/coreclr I'm using is: |
|
@benaadams For visibility and tracking purposes it would be best to open a different issue and add the details there. It appears that the requirements are different for your issue to repro compared to #5077. If creating a small repro project isn't practical (and for your current debugging), try collecting ETW logs: https://github.com/dotnet/corefx/blob/master/Documentation/debugging/windows-instructions.md |
Porting async implementation and adding TPL overrides for SslStream Read/WriteAsync Commit migrated from dotnet/corefx@fac18d8


Fixes #5077
@stephentoub @davidsh @bartonjs @vijaykota PTAL
/cc @xanather, @benaadams @leecow
After review, I'll follow the internal process for RC2.