Merge pull request #1537 from TimSBSquare/bugfix/existing-channel-race-condition

Fixed a race condition when attaching to an existing socket

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