summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTim Stableford <tims@bsquare.com>2021-03-23 12:58:49 +0000
committerTim Stableford <tims@bsquare.com>2021-03-30 14:32:04 +0100
commitef27628c6dff6120b0ed0d4728cc6e8a32b7be53 (patch)
tree0b6eff930ae6d6307f9be739dc546bf6ab1acaab
parent89f9ac00166f1106e03d46562ffeaa3d8032f399 (diff)
downloadnovnc-ef27628c6dff6120b0ed0d4728cc6e8a32b7be53.tar.gz
Fixed a race condition when attaching to an existing socket
This is an error that presents itself with RTCDataChannel's, I suspect this could not happen with a pre-existing WebSocket. If the remote connection creates a data channel then the local (VNC) side gets a channel created callback. It may also be the case that in that very same tick the socket is also opened and buffered data received. This meant that (in my tests) about 1/3 of the time noVNC would fail to respond to the initial message from the server because it was received and subsequently not handled during that initial tick. Also made the documentation reflect this new behaviour and document the existing behaviour.
-rw-r--r--core/rfb.js13
-rw-r--r--docs/API.md5
2 files changed, 16 insertions, 2 deletions
diff --git a/core/rfb.js b/core/rfb.js
index 876255b..f8eeb51 100644
--- a/core/rfb.js
+++ b/core/rfb.js
@@ -286,8 +286,17 @@ export default class RFB extends EventTargetMixin {
this._sock.on('error', e => Log.Warn("WebSocket on-error event"));
// Slight delay of the actual connection so that the caller has
- // time to set up callbacks
- setTimeout(this._updateConnectionState.bind(this, 'connecting'));
+ // time to set up callbacks.
+ // This it not possible when a pre-existing socket is passed in and is just opened.
+ // If the caller creates this object in the open() callback of a socket and there's
+ // data pending doing it next tick causes a packet to be lost.
+ // This is particularly noticable for RTCDataChannel's where the other end creates
+ // the channel and the client, this end, gets notified it exists.
+ if (typeof urlOrChannel === 'string') {
+ setTimeout(this._updateConnectionState.bind(this, 'connecting'));
+ } else {
+ this._updateConnectionState('connecting');
+ }
Log.Debug("<< RFB.constructor");
diff --git a/docs/API.md b/docs/API.md
index aa5aea7..81b517b 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -168,6 +168,11 @@ connection to a specified VNC server.
**`urlOrDataChannel`**
- A `DOMString` specifying the VNC server to connect to. This must be
a valid WebSocket URL. This can also be a `WebSocket` or `RTCDataChannel`.
+ If a `DOMString` is supplied then the connection will be delayed until the next tick to
+ allow allow adding event listeners that fire on connection. If an existing object
+ is supplied then the connection logic happens the same tick. For instance if passing
+ in an existing open WebSocket then it will not be possible to listen for the `connect`
+ event. This is to avoid dropping data on a connection that has data as soon as its opened.
**`options`** *Optional*
- An `Object` specifying extra details about how the connection