Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sometimes RemotingSession closed due to loss of connection in WatsonTCP #75

Open
sancheolz opened this issue Nov 22, 2024 · 12 comments
Open

Comments

@sancheolz
Copy link
Contributor

Describe the bug
With intensive data transfer, an incomplete packet may arrive in the socket, but WatsonTCP does not try to receive it completely. Related to dotnet/WatsonTcp#299

@yallie
Copy link
Collaborator

yallie commented Nov 24, 2024

Have you seen this issue in a real world scenario?

PS. WatsonTCP doesn't seem to be very active lately.
Issues not answered for months, pull requests not evaluated, no commits since May.
Maybe we should consider forking the library and/or using its source code as a
git submodule so that we could fix these bugs ourselves...

@sancheolz
Copy link
Contributor Author

yes, this happens regularly in my project. This test is closest to this situation. dotnet/WatsonTcp#263 . But it is not always reproduced - it is necessary to select the size of the package on which it does not arrive completely. I have already added the WatsonTCP sources to my project and fixed them. Due to the lack of activity in WatsonTCP, you can return to websocket again. Moreover, the problem with the low transfer rate has now also been solved there. Is websocket implementation has hidden issues?

@yallie
Copy link
Collaborator

yallie commented Nov 26, 2024

I have already added the WatsonTCP sources to my project and fixed them

I was suggesting something like that for CoreRemoting.

Is websocket implementation has hidden issues?

I'm not sure...
I've heard websockets have some overhead compared to raw TCP.
Although I haven't seen any benchmarks on the subject.

@sancheolz
Copy link
Contributor Author

WatsonTCP is not raw TCP. It uses framing to support asynchronous sending messages wich not fits to single send and socket buffer size. It adds json header before user data for framing. So it has overhead too. As issues I mean exceptions at closing connections or in Dispose implementation like in WatsonTCP.

If websocket also support framing, then I completely agree with your idea to use the implementation of the websocket from .net.

@yallie
Copy link
Collaborator

yallie commented Nov 26, 2024

WatsonTCP is not raw TCP. It uses framing

Absolutely.

UPD. There is a fork of WatsonTCP that uses binary encoding instead of JSON.
This JSON header is probably the only reason for System.Text.Json dependency.

If websocket also support framing...

As far as I understand, yes, websockets use framing.

https://en.wikipedia.org/wiki/WebSocket#Frame-based_message
https://socketzone.com/2633/websocket-frame
https://noio-ws.readthedocs.io/en/latest/overview_of_websockets.html

A frame looks like this:

 0                   1                   2                   3
 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-------+-+-------------+-------------------------------+
|F|R|R|R| opcode|M| Payload len |    Extended payload length    |
|I|S|S|S|  (4)  |A|     (7)     |             (16/64)           |
|N|V|V|V|       |S|             |   (if payload len==126/127)   |
| |1|2|3|       |K|             |                               |
+-+-+-+-+-------+-+-------------+ - - - - - - - - - - - - - - - +
|     Extended payload length continued, if payload len == 127  |
+ - - - - - - - - - - - - - - - +-------------------------------+
|                               |Masking-key, if MASK set to 1  |
+-------------------------------+-------------------------------+
| Masking-key (continued)       |          Payload Data         |
+-------------------------------- - - - - - - - - - - - - - - - +
:                     Payload Data continued ...                :
+ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +
|                     Payload Data continued ...                |
+---------------------------------------------------------------+

@theRainbird
Copy link
Owner

I would also prefer Websockets over TCP, because it is a well defined standard. TCP is very low level and all higher protocol features must be build individually.

Implementing a websocket chanel from scratch using System.Net.Websockets sounds like the best option. When its working, we can drop websocket-sharp.

At the end is important, that CoreRemoting has one reliable and fast transport channel, not many ones with issues.

@theRainbird
Copy link
Owner

I've googled a bit about using System.Net.Websockets with .NET Framework 4.8.
It seems that websockets are not fully implemented in .NET Framework 4.8. The websocket handshake must be implemented manually.

Here an AI generated example for a websocket server that should run on .NET Framework 4.8 with manual handshake:

using System;
using System.IO;
using System.Net;
using System.Security.Cryptography;
using System.Text;
using System.Threading;
using System.Net.WebSockets;
using System.Threading.Tasks;

class Program
{
    static void Main(string[] args)
    {
        // Start the HTTP listener
        HttpListener listener = new HttpListener();
        listener.Prefixes.Add("http://localhost:8080/");
        listener.Start();
        Console.WriteLine("Server running on http://localhost:8080/");

        while (true)
        {
            // Accept incoming HTTP requests
            HttpListenerContext context = listener.GetContext();
            ThreadPool.QueueUserWorkItem(HandleClient, context);
        }
    }

    static void HandleClient(object obj)
    {
        HttpListenerContext context = (HttpListenerContext)obj;
        HttpListenerRequest request = context.Request;
        HttpListenerResponse response = context.Response;

        // WebSocket handshake - check if this is a WebSocket upgrade request
        if (request.Headers["Upgrade"] == "websocket" && request.Headers["Connection"] == "Upgrade")
        {
            string secWebSocketKey = request.Headers["Sec-WebSocket-Key"];
            string acceptKey = GenerateWebSocketAcceptKey(secWebSocketKey);

            // WebSocket Handshake Response
            response.StatusCode = 101; // Switching Protocols
            response.Headers.Add("Upgrade", "websocket");
            response.Headers.Add("Connection", "Upgrade");
            response.Headers.Add("Sec-WebSocket-Accept", acceptKey);
            response.Close();

            // After handshake, use System.Net.WebSockets WebSocket for communication
            HandleWebSocketCommunication(context);
        }
        else
        {
            response.StatusCode = 400; // Bad Request
            response.Close();
        }
    }

    // WebSocket communication - reading and writing messages using System.Net.WebSockets
    static async void HandleWebSocketCommunication(HttpListenerContext context)
    {
        // Upgrade the connection to a WebSocket using the HTTP connection
        using (NetworkStream stream = context.Connection.GetStream())
        {
            var webSocket = new WebSocketAdapter(stream); // We need to create a custom WebSocket adapter

            // Begin reading from WebSocket (this requires async)
            await ReceiveWebSocketMessages(webSocket);
        }
    }

    // Receive messages from the WebSocket (using System.Net.WebSockets)
    static async Task ReceiveWebSocketMessages(WebSocket webSocket)
    {
        byte[] buffer = new byte[1024];
        while (webSocket.State == WebSocketState.Open)
        {
            WebSocketReceiveResult result = await webSocket.ReceiveAsync(new ArraySegment<byte>(buffer), CancellationToken.None);

            if (result.MessageType == WebSocketMessageType.Text)
            {
                string message = Encoding.UTF8.GetString(buffer, 0, result.Count);
                Console.WriteLine("Message received: " + message);

                // Send a message back
                await webSocket.SendAsync(new ArraySegment<byte>(Encoding.UTF8.GetBytes("Echo: " + message)), WebSocketMessageType.Text, true, CancellationToken.None);
            }
        }
    }

    // Generate the WebSocket Accept key from the Sec-WebSocket-Key (handshake)
    static string GenerateWebSocketAcceptKey(string secWebSocketKey)
    {
        // The Accept key is generated by concatenating the Sec-WebSocket-Key with a magic GUID and hashing it
        string combined = secWebSocketKey + "258EAFA5-E914-47DA-95CA-C5AB0DC85B11";
        using (SHA1 sha1 = SHA1.Create())
        {
            byte[] hash = sha1.ComputeHash(Encoding.UTF8.GetBytes(combined));
            return Convert.ToBase64String(hash);
        }
    }
}

// WebSocketAdapter - This is a custom adapter class for wrapping NetworkStream as a WebSocket
class WebSocketAdapter : WebSocket
{
    private readonly NetworkStream _stream;

    public WebSocketAdapter(NetworkStream stream)
    {
        _stream = stream;
    }

    public override WebSocketState State => WebSocketState.Open;

    public override Task CloseAsync(WebSocketCloseStatus closeStatus, string statusDescription, CancellationToken cancellationToken)
    {
        // Implement WebSocket close logic here
        return Task.CompletedTask;
    }

    public override Task CloseOutputAsync(WebSocketCloseStatus closeStatus, string statusDescription, CancellationToken cancellationToken)
    {
        // Implement WebSocket close output logic here
        return Task.CompletedTask;
    }

    public override Task SendAsync(ArraySegment<byte> buffer, WebSocketMessageType messageType, bool endOfMessage, CancellationToken cancellationToken)
    {
        // Send the message through the NetworkStream
        _stream.Write(buffer.Array, buffer.Offset, buffer.Count);
        return Task.CompletedTask;
    }

    public override Task<WebSocketReceiveResult> ReceiveAsync(ArraySegment<byte> buffer, CancellationToken cancellationToken)
    {
        // Read the data from the NetworkStream
        int bytesRead = _stream.Read(buffer.Array, buffer.Offset, buffer.Count);
        return Task.FromResult(new WebSocketReceiveResult(bytesRead, WebSocketMessageType.Text, true));
    }
}

@yallie
Copy link
Collaborator

yallie commented Nov 27, 2024

Thanks for the reply!
I'm surprized it takes so much effort to start up websocket server.

Here is a small sample that I just tested on .NET 4.8.
Based on this stackoverflow answer: https://stackoverflow.com/a/49843604/544641
As you can see, server part is much smaller than your AI-generated sample.
And it looks like it handles ws handshake by itself.
The browser wouldn't connect to the server if it didn't send the handshake.

Server running .NET 4.8:
image

Browser:
image

C# server:

using System;
using System.Net;
using System.Net.WebSockets;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using System.Runtime.InteropServices;

namespace WebSocketServerConsole
{
    public class Program
    {
        static HttpListener httpListener = new HttpListener();
        private static Mutex signal = new Mutex();

        public static async Task Main(string[] args)
        {
            httpListener.Prefixes.Add("http://localhost:8080/");
            httpListener.Start();

            Console.WriteLine($"Version: {Environment.Version}");
            Console.WriteLine($"RuntimeInformation.FrameworkDescription: {RuntimeInformation.FrameworkDescription}");
            Console.WriteLine($"Server started. Open test1.html to connect...");

            while (signal.WaitOne())
            {
                await ReceiveConnection();
            }

            Console.WriteLine("Server stopped! Bye.");
        }

        public static async System.Threading.Tasks.Task ReceiveConnection()
        {
            HttpListenerContext context = await httpListener.GetContextAsync();

            if (context.Request.IsWebSocketRequest)
            {
                HttpListenerWebSocketContext webSocketContext = await context.AcceptWebSocketAsync(null);
                WebSocket webSocket = webSocketContext.WebSocket;

                while (webSocket.State == WebSocketState.Open)
                {
                    var msg = $"Hello world! {DateTime.Now}";
                    Console.WriteLine($"Sending: {msg}");

                    await webSocket.SendAsync(new ArraySegment<byte>(Encoding.UTF8.GetBytes(msg)),
                        WebSocketMessageType.Text, true, CancellationToken.None);

                    await Task.Delay(TimeSpan.FromSeconds(1.5));
                }
            }

            signal.ReleaseMutex();
        }
    }
}

Javascript client:

<!DOCTYPE html>
  <meta charset="utf-8" />
  <title>WebSocket Test</title>
  <script language="javascript" type="text/javascript">

  var wsUri = "ws://localhost:8080/";
  var output;

  function init()
  {
    output = document.getElementById("output");
    testWebSocket();
  }

  function testWebSocket()
  {
    websocket = new WebSocket(wsUri);
    websocket.onopen = function(evt) { onOpen(evt) };
    websocket.onclose = function(evt) { onClose(evt) };
    websocket.onmessage = function(evt) { onMessage(evt) };
    websocket.onerror = function(evt) { onError(evt) };
  }

  function onOpen(evt)
  {
    writeToScreen("CONNECTED");
    doSend("WebSocket rocks");
  }

  function onClose(evt)
  {
    writeToScreen("DISCONNECTED");
  }

  function onMessage(evt)
  {
    writeToScreen('<span style="color: blue;">RESPONSE: ' + evt.data+'</span>');
  }

  function onError(evt)
  {
    writeToScreen('<span style="color: red;">ERROR:</span> ' + evt.data);
  }

  function doSend(message)
  {
    writeToScreen("SENT: " + message);
    websocket.send(message);
  }

  function writeToScreen(message)
  {
    var pre = document.createElement("p");
    pre.style.wordWrap = "break-word";
    pre.innerHTML = message;
    output.appendChild(pre);
  }

  window.addEventListener("load", init, false);

  </script>

  <h2>WebSocket Test</h2>

  <div id="output"></div>

@yallie
Copy link
Collaborator

yallie commented Nov 28, 2024

I've implemented a basic client/server pair based on System.Net.Websockets.
They interoperate well with old clients and server based on WebSocketSharp.
I tried all possible combinations, looks like they just work.

But what really puzzles me is that:

image

System.Net.Websockets tests run on my machine ~4 times faster than WebSocketSharp tests.
It seems that WebSocketSharp-based client is ok, but its server is slower than HttpListener-based one.

@theRainbird
Copy link
Owner

That's very good news. 👍
Then my worries were unfounded 😄

websocket-sharp uses TcpListener: https://github.com/sta/websocket-sharp/blob/master/websocket-sharp/Server/WebSocketServer.cs

@sancheolz
Copy link
Contributor Author

sancheolz commented Nov 29, 2024

Amazing. System.Net.Websockets, is even faster than WatsonTCP. Could you check delivery of large messages (~2 MB) wich not fits to one frame?

Never mind. Judging by the commits, this has already been done.

In 2 weeks I will be able to check the new channel on my benchmark.
Thanks.

@yallie
Copy link
Collaborator

yallie commented Nov 30, 2024

Could you check delivery of large messages (~2 MB) wich not fits to one frame?

I've added a unit test for large messages:

public void Large_messages_are_sent_and_received()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants