[wasm] WebSocket improve send/receive buffers

[pavelsavara]

• Eliminate as many copies of the data as possible, copy data synchronously when possible
• move buffers to js side
• move most of the implementation to js
• when send buffer became larger than 64k, resolve "send" Task only after data departed to OS via bufferedAmount === 0
• resolve "close" Task only after we received "close" response from browser
• introduce simple cancelable promise on JS side
• Abort() cancels all the pending send/receive/open/close Tasks.
• Abort() sends "close" to server, but doesn't wait for response
• The cancelationToken cancels only the current operation and sets the whole WS into Aborted state. Other pending operations would finish normally but BrowserWebSocket would throw on any subsequent operations, except "close".

The previous BrowserWebSocket implementation did following actions, which the proposed implementation doesn't do

• SendAsync didn't block task on endOfMessage for large pending transmission
• CloseOutputAsync self produced onClose message for it's receivers
• ConnectAsync sent "close" message to server and self produced onClose message for it's receivers, when canceled
• ReceiveAsync sent "close" message to server , when canceled

Fixes #44614
Fixes #58202
Makes #57519 to just report slow test to console instead of fail.
Makes #53957 to just report slow test to console instead of fail.
Fixes #58031

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

• Eliminate as many copies of the data as possible, copy data synchronously
• resolve Task/promise only after data departed to OS via bufferedAmount === 0
• throw when "Exactly one send and one receive is supported on each object in parallel." is breached

Fixes #44614

Author:	pavelsavara
Assignees:	-
Labels:	arch-wasm, area-System.Net
Milestone:	-

[pavelsavara]

Right now it doesn't handle all the abort/cancelation/dispose scenarios. Work in progress ... stay tuned :-D

[pavelsavara]

I tested with Chrome, Edge and Firefox locally.

[kg]

Why does this PR remove Add/RemoveEventListener?

[pavelsavara]

Why does this PR remove Add/RemoveEventListener?

Because we don't need it anymore. Do you think that it should be part of the public API going forward ?

[pavelsavara]

The implementation in the PR right now is running unit test suite 5x slower than main branch. It's caused by blocking send operation until the buffer is empty.

We had chat with @kg and here are conclusions

• we would not block send operation's Task if the bufferedAmount is under 64k. This would improve throughput nearly to previous implementation and also apply backpressure for apps sending large data.
• we would keep the Add/RemoveEventListener code
• we would not automatically send 'close' to the server on operation cancelation. But we should allow the user to use CloseAsync after any cancelation.

Edit: this is all done now

[kg]

Looks good other than comments (you don't need to fix all of them)

[pavelsavara]

Test failures are unrelated

[pavelsavara]

Notes I made during code walkthrough with @CarnaViire

• OutOfMemory exception, it should not be translated as WebSocketException
• order of response ints[] on receive, name the index constants
• rename promise_control for open
• ws[BINDING.wasm_ws_pending_receive_promise_queue].drain -> int[]
• fixed GC root for middle of array, use ArraySegment and offset instead
• _mono_wasm_web_socket_send_buffering message_ptr-> message_root
• utf8 test outerloop on coreCLR
• bufferedAmount: "This value does not reset to zero when the connection is closed" what about CloseSent ?
• assert: when we get message and the receivers queue is not empty, there need to be empty even queue,
• assert: mirror of above in the receiver method
• code 1000 on abort https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent/code
  • 1005/1006 "is a reserved value and MUST NOT be set as a status code in a Close control frame by an endpoint." RFC6455
  • Chrome exception on using 1001: "The code must be either 1000, or between 3000 and 4999", same for no (optional) code
• SendReceive_VaryingLengthBuffers_Success - no change for CoreCLR, keep the active issue for why it's so slow

[pavelsavara]

Follow up on yesterday conversation with @CarnaViire about CloseOutput from client side.
There is no such API on browser WS.
And therefore we call WebSocket.close() instead.
Chrome immediately issues 'close' event and it would not call and any 'message' callbacks after that, even if there were some messages in flight from the server.

So, we are losing the messages from server after CloseOutputAsync, as compared to managed/desktop implementation which delivers them.

The alternative is to not call WebSocket.close() when CloseOutputAsync, but that seems even worse.

[kg]

Follow up on yesterday conversation with @CarnaViire about CloseOutput from client side.
There is no such API on browser WS.
And therefore we call WebSocket.close() instead.
Chrome immediately issues 'close' event and it would not call and any 'message' callbacks after that, even if there were some messages in flight from the server.

So, we are losing the messages from server after CloseOutputAsync, as compared to managed/desktop implementation which delivers them.

The alternative is to not call WebSocket.close() when CloseOutputAsync, but that seems even worse.

Hm, this is a huge failure on browser vendors' part. I don't like the data loss here. Is there some way we could signal this behavior difference to the developer?

[pavelsavara]

The other differences in behavior are

• we are not streaming partial messages, we delay them till endOfMessage.
• we are sending close(1000) NormalClosure on Abort()

[pavelsavara]

Is there some way we could signal this behavior difference to the developer?

We agreed with @kg to issue warning into console once per app lifetime.
WARNING: Web browsers do not support closing the output side of a WebSocket. CloseOutputAsync has closed the socket and discarded any incoming messages.