StreamSocketOutput WriteAsync as async (SSL) #588
Conversation
benaadams
changed the title
benaadams
force-pushed
the
async-write
branch
from
6543ff7 to
d3a54d6
Compare
|
One half dotnet/corefx#5505 though I think it actually needs the ReadAsync also (as a bidirectional stream and I saw blocking on Read) |
|
Also this is relevant dotnet/coreclr#2724 |
| } | ||
| if (!endTask.IsCompleted) | ||
| { | ||
| return endTask; |
There was a problem hiding this comment.
Is it wrong to simply return endTask if it's not completed? Will it ever complete prior to preambleTask or dataTask?
There was a problem hiding this comment.
That's a good point, no
benaadams
force-pushed
the
async-write
branch
from
3127b84 to
0ec69fd
Compare
| block = _producingBlock; | ||
| while (block != end.Block) | ||
| { | ||
| _outputStream.WriteAsync(block.Data.Array, block.Data.Offset, block.Data.Count, CancellationToken.None); |
There was a problem hiding this comment.
What if the wrapping stream produces an async error? I think we have to await.
There was a problem hiding this comment.
Will have to await them one after another too as don't know the characteristics of the underlying stream; and each write will be sent with immediate:true ;-(
There was a problem hiding this comment.
Also means have to do .GetAwaiter().GetResult() else the function may return in an await point... but will only be one.
benaadams
changed the title
|
Last item dotnet/corefx#5541 has just been merged - will need feed update |
benaadams
force-pushed
the
async-write
branch
2 times, most recently
from
4eb8f90 to
8aa5dd0
Compare
benaadams
changed the title
|
Still an issue here - investigating |
|
👀 |
|
@muratg this is something I really want - however there have been so many strange issues with it all the way down to coreclr and cross framework; I wouldn't feel confident with it going in rushed - maybe an item to pair with WebSockets timeline? (as the current workaround does work, with caveats) |
|
e.g. backlog |
|
What are we doing with this? Is it still blocked on corefx? |
|
It was still jamming when I last tried it; but I haven't had time to debug to find the problem. SslStream doesn't implement true A change was made to corefx so these methods didn't become sync in coreclr with read and write blocking each other - however I don't think that entirely fixed the issue and I observed something similar happening in full framework. However; that may have just been a point in time feed issue. |
|
Looks like this might be the final blocker dotnet/corefx#7800 |
benaadams
changed the title
|
Rebased, still waiting for dotnet/corefx#7800 and/or dotnet/corefx#7819 to hit the feed to likely work |
| { | ||
| _outputStream.Write(block.Data.Array, block.Data.Offset, block.Data.Count); | ||
| ProducingCompleteAsync(end).GetAwaiter().GetResult(); |
There was a problem hiding this comment.
I wonder if it's better not to wait for the writes to complete in ProducingComplete. The real SocketOutput.ProducingComplete doesn't block.
There was a problem hiding this comment.
Its a sync memory write though? Issue is the unknown stream type
There was a problem hiding this comment.
The main difference is that the real ProducingComplete would never wait for the write behind buffer to flush. This might by waiting for the Task returned by SocketOutput.WriteAsync. It's a difference in behavior that we probably don't account for in the calling code.
| } | ||
|
|
||
| block = _producingBlock; | ||
| while (block != end.Block) |
There was a problem hiding this comment.
I like this way of ensuring blocks are returned better than the _canWrite flag in #750
There was a problem hiding this comment.
The purpose of _canWrite is to prevent further attempts to call Write().
|
Probably needs to rebase on #750 as this is waiting for a corefx fix to hit the feeds |
|
@benaadams #750 now in. |
|
Issue on full framework (block) |
|
Works on coreclr; have ifdef'd for full framework Raise issue for full framework? |
benaadams
changed the title
|
Issue raised for full framework #768 |
| var beginChunkBytes = ChunkWriter.BeginChunkBytes(buffer.Count); | ||
|
|
||
| await _outputStream.WriteAsync(beginChunkBytes.Array, beginChunkBytes.Offset, beginChunkBytes.Count, cancellationToken); | ||
| await _outputStream.WriteAsync(buffer.Array ?? _nullBuffer, buffer.Offset, buffer.Count, cancellationToken); |
There was a problem hiding this comment.
buffer.Array ?? _nullBuffer: the coalescing part of this expression will never be hit. buffer.Array is checked in WriteAsync
|
Bummer that it doesn't work on desktop clr, but that's better than nothing 😄
|
This prevents https from being async
Blocked by https://github.com/dotnet/corefx/issues/5488 / https://github.com/dotnet/corefx/issues/5077
Blockers
Resolves #569
Works on coreclr, ifdef'd for full framework