Runtime: Use managed WebSockets implementation on all Windows (not UWP)

[stephentoub]

Now that we have a managed implementation of Websockets that we use on Unix, should we also use it on Windows pre-Windows 8, since the current Windows implementation requires OS support only available Windows 8 and later? I'm assuming this should mostly be an issue of package authoring?

cc: @davidsh, @cipop, @ericstj, @davidfowl

[ghost]

@stephentoub, (bit off-topic) is there a major performance difference when switching to managed version of WebSockets on Windows 10, for example?

[stephentoub]

(bit off-topic) is there a major performance difference when switching to managed version of WebSockets on Windows 10, for example?

See some numbers in my PR here:
https://github.com/dotnet/corefx/pull/9412

[meriturva]

Any news here? is it planed to be released on milestone 1.2.0 ? April 2017 ? I think that we need a solution now for Windows 7! What do you think?

[CIPop]

@stephentoub I believe it's good to have an implementation of WebSockets for Win7 as long as this implementation is not replacing the WinHttp based one for Win8+.
The only downside is that the managed implementation is lacking a full implementation of the HTTP protocol required for the upgrade. To set the correct expectations: do you have a feature comparison table between the managed and WinHttp-based implementations?

[stephentoub]

I believe it's good to have an implementation of WebSockets for Win7 as long as this implementation is not replacing the WinHttp based one for Win8+

@ericstj, we can specialize the package in this way, right? i.e. have a build for Win7 and a different one for Win8+?

To set the correct expectations: do you have a feature comparison table between the managed and WinHttp-based implementations?

I don't, but if you list out the features, I can share which the managed does/doesn't currently support. Some of the limitations are things it looks like the WinHTTP implementation isn't respecting anyway, e.g. we don't currently pay attention to the ClientWebSocketOptions.Proxy in the managed implementation (should be relatively straightforward to add, though), but then looking at the WinHTTP implementation, it appears to ignore it as well.

[CIPop]

I don't, but if you list out the features, I can share which the managed does/doesn't currently support

Answering this is definitely not an easy task given the large RFCs for both HTTP/S and WebSockets. Here are a few of WinHTTPs features as well as a link to the WS RFC: https://msdn.microsoft.com/en-us/library/windows/desktop/aa382925(v=vs.85).aspx

some of the limitations are things it looks like the WinHTTP implementation isn't respecting anyway, e.g. we don't currently pay attention to the ClientWebSocketOptions.Proxy

This is correct and the expectation is set via the following issues: dotnet/runtime#15994, dotnet/runtime#14911.
If I correctly recall, the contract is also missing a few tests and requires further unification with WinHttpHandler. /cc: @davidsh

[CIPop]

I've added my idea of an implementation plan in https://github.com/dotnet/corefx/issues/13663#issuecomment-265879275. The plan can be split to handle this item (leave the code in S.N.WebSocket.Client but add an OS platform check).

Since we already have some of the testing done and most code shouldn't change, working on this item should be fairly easy (but adding extra tests is always welcomed). Requires Win7/ build -outerloop testing.

[JustArchi]

Bump, is there any progress on this? We've stumbled upon this exact issue in dotnet/corefx#24249 as well as https://github.com/SteamRE/SteamKit/issues/455. It'd probably be a good idea to address this.

[MikhailArkhipov]

We really need this to support R in VS Code. Many R users are on Win7

[ghost]

Please issue a managed websocket package for Windows Server 2008 R2 and one for Windows 7.
Can it be made available in 2.1 time frame?

[wsnell]

Desperately need

[TechnikEmpire]

If people are in a crunch there are a bazillion pure managed implementations out there. My issue is that the kestrel websocket add-on fails due to this bug, so I now have to write my own plugin for kestrel.

[stephentoub]

there are a bazillion pure managed implementations out there

The implementation that's used here on Unix is managed, built on sockets.

[davidfowl]

Kestrel doesn’t fail due to this. Do you mean kestrel with IIS fails?

[TechnikEmpire]

@davidfowl No I meant just straight up kestrel. I have a library that uses Kestrel to run a local transparent filtering proxy and it collapses on Windows 7 with platform not supported exception because I use the kestrel websocket addon.

[TechnikEmpire]

2017-11-24 16:31:12.7066 INFO Websocket client accepted.     FilterWebsocketHandler.cs::Handle() dotnet/runtime#13876
2017-11-24 16:31:12.7066 ERROR The WebSocket protocol is not supported on this platform.   at System.Net.WebSockets.WebSocketHelpers.ThrowPlatformNotSupportedException_WSPC()
   at System.Net.WebSockets.ClientWebSocket..ctor()
   at CitadelCore.Net.Handlers.FilterWebsocketHandler.<Handle>d__1.MoveNext()
     From FilterWebsocketHandler.cs::Handle() dotnet/runtime#13918
2017-11-24 16:31:13.1046 INFO Accepting websocket client.     FilterWebsocketHandler.cs::Handle() dotnet/corefx#61
2017-11-24 16:31:13.1046 INFO Microsoft.AspNetCore.WebSockets.Protocol.CommonWebSocket     FilterWebsocketHandler.cs::Handle() dotnet/runtime#13874
2017-11-24 16:31:13.1046 INFO Websocket client accepted.     FilterWebsocketHandler.cs::Handle() dotnet/runtime#13876
2017-11-24 16:31:13.1046 ERROR The WebSocket protocol is not supported on this platform.   at System.Net.WebSockets.WebSocketHelpers.ThrowPlatformNotSupportedException_WSPC()
   at System.Net.WebSockets.ClientWebSocket..ctor()
   at CitadelCore.Net.Handlers.FilterWebsocketHandler.<Handle>d__1.MoveNext()
     From FilterWebsocketHandler.cs::Handle() dotnet/runtime#13918

Windows 7 x64 home and pro, all up to date.

[TechnikEmpire]

Ah my bad it looks like it's failing on the creation of WebSocket, not on the acceptance of the kestrel websocket (they are not the same class). My bad @davidfowl

[weshaggard]

We should try and get this work done for 2.1 and repackage the IL only version for win7.

[wsnell]

Yes please do. Also full support for duplex contracts using websockets too.

[TechnikEmpire]

@weshaggard good call. Somebody give this guy a promotion.

[weshaggard]

@stephentoub @davidsh is there any reason we cannot just use the IL only implementation on all platforms? That will make fixing this a lot easier.

[stephentoub]

is there any reason we cannot just use the IL only implementation on all platforms?

Personally I'd be happy with that. However there may be some functional gaps we're not aware of, things that WinHTTP is providing that the managed implementation currently doesn't have (I don't have any examples, though).
cc: @geoffkizer

[davidsh]

@stephentoub @davidsh is there any reason we cannot just use the IL only implementation on all platforms? That will make fixing this a lot easier.

The "pure managed" implementation of WebSockets.Client is missing features compared to the current Windows implementation (which uses native WinHTTP). So, it is not ready to be used as the replacement. Also, the UWP version of WebSockets.Client currently uses WinRT WebSocket APIs. So, we can't just remove those implementations yet since the managed one is not at feature parity.

[stephentoub]

The "pure managed" implementation of WebSockets.Client is missing features

@davidsh, which features specifically?

[davidsh]

proxy and HTTP authentication for starters.

[stephentoub]

proxy and HTTP authentication for starters.

I don't see the Windows implementation accessing the relevant members of ClientWebSocketOptions... can you point me to where that's happening? i.e. where is it accessing Credentials/_credentials or Proxy/_proxy?

[wsnell]

Honestly I don’t care if some features are missing (that is better than nothing). Let’s simply add support for Win7 as a managed code and by checking platform, upgrade to a system provided solution on win8+, etc. This is much better than not all that great third party solutions that also have licensing issues for many people.
Thanks
-Wes

[EDIT] @karelz removed email reply

[weshaggard]

Given that in 2.0 this ships in the shared framework instead of an individual library package it will be tricky to have a win7 and win10 specific implementation because we only produce one shared framework for windows which is the portable win one. I don't think we want to complicate everything else consuming the .NET Core shared framework on windows to not start shipping 2 just for this library so we need another solution.

Possible solutions I see are:
1) Ship a win7 and win10 specific .NET Core shared framework.
2) Start shipping the library package again.
3) Use the same IL only implementation on all windows .NET Core platforms (we can still have a different one for UWP).
4) Ship one library on windows and do runtime detection to chose the right implementation

I don't think we want option (1) and option (2) will have various complications and folks will have to explicitly opt-in by adding that reference. So I think either (3) or (4) is our best option.

[davidsh]

I don't see the Windows implementation accessing the relevant members of ClientWebSocketOptions... can you point me to where that's happening? i.e. where is it accessing Credentials/_credentials or Proxy/_proxy?

It might not be using it right now since we have the other issue with ClientWebSocketOptions not being implemented. But adding that code would simply use the built-in mechanisms for proxy and HTTP auth for the Windows platforms.

I suppose in the scheme of things it might not matter. If someone wants to do an experiment with removing the WinHTTP version for Windows and simply using the managed WebSockets.Client everywhere (except for UWP), we can see where we land in terms of test failures, etc.

[davidsh]

Possible solutions I see are:
Use the same IL only implementation on all windows .NET Core platforms (we can still have a different one for UWP).

I see number 3 as the current goal for now.

[stephentoub]

If someone wants to do an experiment with removing the WinHTTP version for Windows and simply using the managed WebSockets.Client everywhere (except for UWP), we can see where we land in terms of test failures, etc.

FWIW, that's a very easy experiment to do. The src .csproj contains this:

    <!-- // uncomment to use the managed "unix" implementation on Windows
      <TargetsWindows>false</TargetsWindows>
      <TargetsUnix>true</TargetsUnix>
    -->

By changing it to:

      <TargetsWindows>false</TargetsWindows>
      <TargetsUnix>true</TargetsUnix>
    <!-- // uncomment to use the managed "unix" implementation on Windows
    -->

and rebuilding, then running the tests, I get:

  Discovering: System.Net.WebSockets.Client.Tests
  Discovered:  System.Net.WebSockets.Client.Tests
  Starting:    System.Net.WebSockets.Client.Tests
     System.Net.WebSockets.Client.Tests.MemorySendReceiveTest.SendReceive_ConnectionClosedPrematurely_ReceiveAsyncFailsAndWebSocketStateUpdated [FAIL]
        Assert.Equal() Failure
        Expected: 2147954431
        Actual:   2147500037
        Stack Trace:
           c:\Users\stoub\Source\Repos\corefx\src\System.Net.WebSockets.Client\tests\SendReceiveTest.cs(477,0): at System.Net.WebSockets.Client.Tests.SendReceiveTest.<>c__DisplayClass13_0.<<SendRe
  ceive_ConnectionClosedPrematurely_ReceiveAsyncFailsAndWebSocketStateUpdated>b__0>d.MoveNext()
           --- End of stack trace from previous location where exception was thrown ---
           c:\Users\stoub\Source\Repos\corefx\src\System.Net.WebSockets.Client\tests\SendReceiveTest.cs(492,0): at System.Net.WebSockets.Client.Tests.SendReceiveTest.<>c__DisplayClass13_0.<<SendRe
  ceive_ConnectionClosedPrematurely_ReceiveAsyncFailsAndWebSocketStateUpdated>b__1>d.MoveNext()
           --- End of stack trace from previous location where exception was thrown ---
           c:\Users\stoub\Source\Repos\corefx\src\Common\tests\System\Net\Http\LoopbackServer.cs(67,0): at System.Net.Test.Common.LoopbackServer.<>c__DisplayClass3_0.<CreateServerAsync>b__0(Task t
  )
              at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
              at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot)
           --- End of stack trace from previous location where exception was thrown ---
           c:\Users\stoub\Source\Repos\corefx\src\System.Net.WebSockets.Client\tests\SendReceiveTest.cs(488,0): at System.Net.WebSockets.Client.Tests.SendReceiveTest.<SendReceive_ConnectionClosedP
  rematurely_ReceiveAsyncFailsAndWebSocketStateUpdated>d__13.MoveNext()
           --- End of stack trace from previous location where exception was thrown ---
           --- End of stack trace from previous location where exception was thrown ---
           --- End of stack trace from previous location where exception was thrown ---
     System.Net.WebSockets.Client.Tests.ArraySegmentSendReceiveTest.SendReceive_ConnectionClosedPrematurely_ReceiveAsyncFailsAndWebSocketStateUpdated [FAIL]
        Assert.Equal() Failure
        Expected: 2147954431
        Actual:   2147500037
        Stack Trace:
           c:\Users\stoub\Source\Repos\corefx\src\System.Net.WebSockets.Client\tests\SendReceiveTest.cs(477,0): at System.Net.WebSockets.Client.Tests.SendReceiveTest.<>c__DisplayClass13_0.<<SendRe
  ceive_ConnectionClosedPrematurely_ReceiveAsyncFailsAndWebSocketStateUpdated>b__0>d.MoveNext()
           --- End of stack trace from previous location where exception was thrown ---
           c:\Users\stoub\Source\Repos\corefx\src\System.Net.WebSockets.Client\tests\SendReceiveTest.cs(492,0): at System.Net.WebSockets.Client.Tests.SendReceiveTest.<>c__DisplayClass13_0.<<SendRe
  ceive_ConnectionClosedPrematurely_ReceiveAsyncFailsAndWebSocketStateUpdated>b__1>d.MoveNext()
           --- End of stack trace from previous location where exception was thrown ---
           c:\Users\stoub\Source\Repos\corefx\src\Common\tests\System\Net\Http\LoopbackServer.cs(67,0): at System.Net.Test.Common.LoopbackServer.<>c__DisplayClass3_0.<CreateServerAsync>b__0(Task t
  )
              at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
              at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot)
           --- End of stack trace from previous location where exception was thrown ---
           c:\Users\stoub\Source\Repos\corefx\src\System.Net.WebSockets.Client\tests\SendReceiveTest.cs(488,0): at System.Net.WebSockets.Client.Tests.SendReceiveTest.<SendReceive_ConnectionClosedP
  rematurely_ReceiveAsyncFailsAndWebSocketStateUpdated>d__13.MoveNext()
           --- End of stack trace from previous location where exception was thrown ---
           --- End of stack trace from previous location where exception was thrown ---
           --- End of stack trace from previous location where exception was thrown ---
  Finished:    System.Net.WebSockets.Client.Tests

  === TEST EXECUTION SUMMARY ===
c:\Users\stoub\Source\Repos\corefx\Tools\tests.targets(484,5): warning :    System.Net.WebSockets.Client.Tests  Total: 104, Errors: 0, Failed: 2, Skipped: 0, Time: 23.857s [c:\Users\stoub\Source\R
epos\corefx\src\System.Net.WebSockets.Client\tests\System.Net.WebSockets.Client.Tests.csproj]

Those two failures are due to this assert:
https://github.com/dotnet/corefx/blob/9a2e5d8f0e7f816f983bce75f5bba22811df4994/src/System.Net.WebSockets.Client/tests/SendReceiveTest.cs#L477
which is validating a WinHTTP error code, so obviously it's going to be different.

Obviously take this with a grain of salt. Our test scenario coverage isn't exactly stellar.

[davidsh]

Ok. Well, then it looks like the fix for this issue is to simply dump the WinHTTP version and use the managed WebSockets.Client implementation everywhere except UWP. Then as we add features for supporting ClientWebSocketOptions, we'll need to add platform-specific code to handle HTTP auth (at least for the hard stuff like Negotiate and NTLM) and proxy detection logic.

I'm not opposed to you doing such a PR and also cleaning up (deleting) the (now) obsolete WinHTTP implementation.

[wsnell]

I was actually proposing [4].

[EDIT] @karelz email reply text removed - https://github.com/dotnet/corefx/issues/9503#issuecomment-348625061

[karelz]

In internal discussion we found out that shipping the package just for Win7 is difficult and problematic. We are going with option [3] above -- use managed WebSockets for all Windows (except UWP). Title updated.

More details on the risk assessment from @davidsh (thanks!)

Both implementations (managed and WinHttp based) have the same feature gaps.
The managed WebSocket implementation (currently used only on Linux) is missing HTTP auth and proxy support.
The Windows .NET Core WebSockets implementation used on Windows 8+ (using WinHTTP) is also not using HTTP auth nor proxy settings.
While it is easier to enable HTTP auth and proxy for the WinHTTP based implementation, it doesn’t help the managed implementation which is used on Linux. In addition, there is a desire to enable Windows 7 .NET Core support for WebSockets which would require the managed implementation as well.
In terms of test support, it is equivalent. The assumption, though, is that any PRs enhancing the managed WebSockets implementation to add HTTP auth and proxy support would include tests.
I have no problem getting rid of the WinHTTP based implementation for Windows WebSocket on .NET Core and converging on the managed implementation.
For now, though, this work will not include the .NET Core UWP implementation of WebSockets which uses yet another different implementation (WinRT WebSockets).

[davidfowl]

😍 this is great news

[ghost]

This is excellent! Lets invest energies in making the ManagedWebSocket the BEST and most performant implementation, zero copy, memory pools and and and.. all the goodness one can possible imagine these days

Btw, how where do we get the preview System.Net.WebSockets package today? When I searched WebSocket on https://dotnet.myget.org/gallery/dotnet-core, I found System.Net.WebSockets 4.4.0-beta-24913-02 year old.. Where can i get the package that was updated like 4 hours ago or yesterday?

[TechnikEmpire]

@kasper3 lol feel free to submit a PR.

[qmfrederik]

@kasper3 if you open the drtails for that package, you should see preview1 builds which are much more recent. You'll want those.

[qmfrederik]

See this comment on how to get the latest build: https://github.com/dotnet/corefx/issues/5120#issuecomment-348617702

[TechnikEmpire]

@qmfrederik How did you manage to make this work on Windows? Maybe I'm thick lately but following the bread crumbs of your comments has not led me to a solution.

[qmfrederik]

@TechnikEmpire What I did was to clone the CoreFX repository, edit src\System.Net.WebSockets.Client\src\System.Net.WebSockets.Client.csproj so that the package always targets Linux (which means using the managed handler):

  <PropertyGroup>
    <ProjectGuid>{B8AD98AE-84C3-4313-B3F1-EE8BD5BFF69B}</ProjectGuid>
    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
    <TargetsWindows>false</TargetsWindows>
    <TargetsUnix>true</TargetsUnix>
  </PropertyGroup>

and then building that project. You'll probably want to build corefx from the root first if you haven't done so already.

It should get you a System.Net.WebSockets.Client.dll library you can use. I copied that dll to the NuGet cache folder so my projects would always use the modified version.