Runtime: Partial send support for ClientWebSocket on UWP

Created on 11 Jul 2017  路  16Comments  路  Source: dotnet/runtime

The UWP implementation of ClientWebSocket (WinRTWebSocket.cs) buffers the entire message before it can pass it to the underlying WinRT MessageWebSocket. This is due to missing support for sending partial messages in WinRT MessageWebSocket.

dotnet/runtime#22311 improved the receive path of partial messages in UWP, but the existing WinRT APIs still lack partial message support in the send path.

System.Net.HttpListener HttpListenerWebSocketTests will hang in UAP mode.

area-System.Net enhancement

Most helpful comment

Latest Windows 10 OS now has partial send support for the WinRT WebSocket APIs. But .NET Core isn't yet using that in the current WebSockets.Client WinRTWebSocket class.

However, the new plan is to switch away from using the WinRT WebSocket APIs and instead use the managed WebSocket implementation for all platforms (including UWP). See: dotnet/corefx#29644

So, closing this issue since we're tracking dotnet/corefx#29644.

All 16 comments

This is the test case causing the hang: ReceiveAsync_ReadWholeBuffer_Success()

``` c#
[ConditionalTheory(nameof(IsNotWindows7))]
[ActiveIssue(17462, TargetFrameworkMonikers.Uap)]
[InlineData(WebSocketMessageType.Text, false)]
[InlineData(WebSocketMessageType.Binary, false)]
[InlineData(WebSocketMessageType.Text, true)]
[InlineData(WebSocketMessageType.Binary, true)]
public async Task ReceiveAsync_ReadWholeBuffer_Success(WebSocketMessageType messageType, bool endOfMessage)
{
HttpListenerWebSocketContext context = await GetWebSocketContext();
await ClientConnectTask;

        const string Text = "Hello Web Socket";
        byte[] sentBytes = Encoding.ASCII.GetBytes(Text);

        await Client.SendAsync(new ArraySegment<byte>(sentBytes), messageType, endOfMessage, new CancellationToken());

        byte[] receivedBytes = new byte[sentBytes.Length];
        WebSocketReceiveResult result = await ReceiveAllAsync(context.WebSocket, receivedBytes.Length, receivedBytes);
        Assert.Equal(messageType, result.MessageType);
        Assert.Equal(endOfMessage, result.EndOfMessage);
        Assert.Null(result.CloseStatus);
        Assert.Null(result.CloseStatusDescription);

        Assert.Equal(Text, Encoding.ASCII.GetString(receivedBytes));
    }

```

@Diego-Perez-Botero could this be related to client-side changes for WebSockets?

The test will only hang when endOfMessage == false

After discussion with @Diego-Perez-Botero, the root cause of this issue is because ClientWebSocket's UWP implementation is using the winRT websocket, which doesn't support sending partial message.
The detailed explanation can be found here: dotnet/runtime#22311

I have a question though, can we disable a test using [Activeissue] of a closed issue? If not, should we re-open dotnet/runtime#22311?

@davidsh

I have a question though, can we disable a test using [Activeissue] of a closed issue? If not, should we re-open dotnet/runtime#22311?

If you need to disable a test forever due to platform differences, you need to use SkipOnTargetFramework attribute and not ActiveIssue attribute. We have many examples of this in other tests such Http.

See: https://github.com/dotnet/corefx/blob/master/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs#L181

for example.

In your case, you would use TargetFrameworkMonikers.Uap to disable that test for UAP forever. Please add an appropriate explanation in the attribute field.

Please add an appropriate explanation in the attribute field.

sure.

As Caesar mentioned, only the test variants where _endOfMessage_ is false are hanging. This is because the UWP implementation of ClientWebSocket (WinRTWebSocket.cs) buffers the entire message before it can pass it to the underlying WinRT MessageWebSocket. This is due to missing support for _sending_ partial messages in WinRT MessageWebSocket.

dotnet/runtime#22311 improved the _receive_ path of partial messages in UWP, but the existing WinRT APIs still lack partial message support in the _send_ path.

IMHO, dotnet/runtime#22311 should remain closed. The _send_ path improvements should be tracked with a different issue (but shouldn't target .NET Standard 2.0).

/cc @DavidGoll
This is a behavior difference from .Net Desktop and .Net Core. Let's keep this open and have an ActiveIssue for now until we decide it's OK to ship as-is or document.

until we decide it's OK to ship as-is or document.

If we ship as-is (and thus have a behavior difference from .NET Framework), we need to document it here:
https://github.com/dotnet/corefx/wiki/ApiCompat

@caesar1995 document this as a compat issue... there will not be a fix for this in RS3, thus not in UWP6.0.

cc: @mconnew

The behavior now is documented by @davidsh : https://github.com/dotnet/corefx/wiki/ApiCompat

Can we close this issue?

Can we close this issue?

I think it's good that we have documented the limitation but I still think fixing this may be valuable for developers in the future. The fix, given the current WinRT implementation, depends on Windows and cannot be achieved by modifying CoreFX so I'm not sure if it's appropriate to track here.

/cc @davidsh @DavidGoll

Latest Windows 10 OS now has partial send support for the WinRT WebSocket APIs. But .NET Core isn't yet using that in the current WebSockets.Client WinRTWebSocket class.

However, the new plan is to switch away from using the WinRT WebSocket APIs and instead use the managed WebSocket implementation for all platforms (including UWP). See: dotnet/corefx#29644

So, closing this issue since we're tracking dotnet/corefx#29644.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

iCodeWebApps picture iCodeWebApps  路  3Comments

Timovzl picture Timovzl  路  3Comments

jzabroski picture jzabroski  路  3Comments

sahithreddyk picture sahithreddyk  路  3Comments

matty-hall picture matty-hall  路  3Comments