Fix negotiations by better protecting access to PeerConnection

erizo_controller/erizoClient/src/Room.js

@@ -272,7 +272,7 @@ const Room = (altIo, altConnectionHelpers, altConnectionManager, specInput) => {
     if (stream && stream.pc && !stream.failed) {
       stream.pc.processSignalingMessage(arg.mess);
     } else {
-      Logger.error(`Failed appyling a signaling message in ${stream.getID()}`);
+      Logger.error(`Failed applying a signaling message in ${stream.getID()}`, arg.mess);
     }
   };
 

erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js

@@ -365,7 +365,8 @@ const BaseStack = (specInput) => {
   });
 
   // We need to protect it against calling multiple times to createOffer.
-  // Otherwise it could change the ICE credentials before calling setLocalDescription the first time in Chrome.
+  // Otherwise it could change the ICE credentials before calling setLocalDescription
+  // the first time in Chrome.
   that.createOffer = firstOfferQueue.protectFunction((isSubscribe = false, forceOfferToReceive = false, streamId = '') => {
     if (!firstOfferCreated) {
       firstOfferQueue.startEnqueuing();

erizo_controller/erizoJS/erizoJSController.js

@@ -104,20 +104,23 @@ exports.ErizoJSController = (threadPool, ioThreadPool) => {
     const connection = node.connection;
     log.debug(`message: closeNode, clientId: ${node.clientId}, streamId: ${node.streamId}`);
 
-    node.close();
-
-    const client = clients.get(clientId);
-    if (client === undefined) {
-      log.debug('message: trying to close node with no associated client,' +
-        `clientId: ${clientId}, streamId: ${node.streamId}`);
-      return;
-    }
+    return new Promise((resolve) => {
+      const client = clients.get(clientId);
+      if (client === undefined) {
+        log.debug('message: trying to close node with no associated client,' +
+          `clientId: ${clientId}, streamId: ${node.streamId}`);
+        return;
+      }
 
-    const remainingConnections = client.maybeCloseConnection(connection.id);
-    if (remainingConnections === 0) {
-      log.debug(`message: Removing empty client from list, clientId: ${client.id}`);
-      clients.delete(client.id);
-    }
+      const remainingConnections = client.maybeCloseConnection(connection.id);
+      if (remainingConnections === 0) {
+        log.debug(`message: Removing empty client from list, clientId: ${client.id}`);
+        clients.delete(client.id);
+      }
+      node.close().then(() => {
+        resolve();
+      });
+    });
   };
 
   that.rovMessage = (args, callback) => {
@@ -307,10 +310,15 @@ exports.ErizoJSController = (threadPool, ioThreadPool) => {
       const subscriber = publisher.getSubscriber(clientId);
       log.info(`message: removing subscriber, streamId: ${subscriber.streamId}, ` +
         `clientId: ${clientId}`);
-      closeNode(subscriber);
-      publisher.removeSubscriber(clientId);
+      closeNode(subscriber).then(() => {
+        log.info(`message: subscriber node Closed, streamId: ${subscriber.streamId}`);
+        publisher.removeSubscriber(clientId);
+        callback('callback', true);
+      });
+    } else {
+      log.warn(`message: removeSubscriber no publisher has this subscriber, clientId: ${clientId}, streamId: ${streamId}`);
+      callback('callback', true);
     }
-    callback('callback', true);
   };
 
   /*

erizo_controller/erizoJS/models/Connection.js

@@ -20,6 +20,8 @@ const CONN_SDP_PROCESSED = 203;
 const CONN_FAILED = 500;
 const WARN_BAD_CONNECTION = 502;
 
+const RESEND_LAST_ANSWER_RETRY_TIMEOUT = 50;
+
 class Connection extends events.EventEmitter {
   constructor(id, threadPool, ioThreadPool, options = {}) {
     super();
@@ -120,19 +122,34 @@ class Connection extends events.EventEmitter {
     this.emit('status_event', info, evt, streamId);
   }
 
+  _retryWithPromise(fn, timeout) {
+    return new Promise((resolve, reject) => {
+      fn().then(resolve)
+      .catch((error) => {
+        if (error === 'retry') {
+          setTimeout(() => {
+            this._retryWithPromise(fn, timeout).then(resolve, reject);
+          }, timeout);
+        } else {
+          // For now we're resolving the promise instead of rejecting it since
+          // we don't handle this error at the moment
+          resolve();
+        }
+      });
+    });
+  }
+
   _resendLastAnswer(evt, streamId, label, forceOffer = false, removeStream = false) {
     if (!this.wrtc || !this.wrtc.localDescription) {
-      return;
+      log.error('message: _resendLastAnswer, this.wrtc or this.wrtc.localDescription are not present');
+      return Promise.reject('fail');
     }
     this.wrtc.localDescription = new SessionDescription(this.wrtc.getLocalDescription());
     const sdp = this.wrtc.localDescription.getSdp(this.sessionVersion);
     const stream = sdp.getStream(label);
     if (stream && removeStream) {
       log.info(`resendLastAnswer: StreamId ${streamId} is stream and removeStream, label ${label}, sessionVersion ${this.sessionVersion}`);
-      setTimeout(() => {
-        this._resendLastAnswer(evt, streamId, label, forceOffer, removeStream);
-      }, 50);
-      return;
+      return Promise.reject('retry');
     }
     this.sessionVersion += 1;
     let message = sdp.toString();
@@ -141,6 +158,7 @@ class Connection extends events.EventEmitter {
     const info = { type: this.options.createOffer || forceOffer ? 'offer' : 'answer', sdp: message };
     log.debug(`message: _resendLastAnswer sending event, type: ${info.type}, streamId: ${streamId}`);
     this.emit('status_event', info, evt, streamId);
+    return Promise.resolve();
   }
 
   init(newStreamId) {
@@ -223,10 +241,12 @@ class Connection extends events.EventEmitter {
       this.wrtc.removeMediaStream(id);
       this.mediaStreams.get(id).close();
       this.mediaStreams.delete(id);
-      this._resendLastAnswer(CONN_SDP, id, label, true, true);
-    } else {
-      log.error(`message: Trying to remove mediaStream not found, id: ${id}`);
+      return this._retryWithPromise(
+        this._resendLastAnswer.bind(this, CONN_SDP, id, label, true, true),
+        RESEND_LAST_ANSWER_RETRY_TIMEOUT);
     }
+    log.error(`message: Trying to remove mediaStream not found, id: ${id}`);
+    return Promise.resolve();
   }
 
   setRemoteDescription(sdp, streamId) {

erizo_controller/erizoJS/models/Publisher.js

@@ -385,6 +385,7 @@ class Source extends NodeClass {
 
   // eslint-disable-next-line class-methods-use-this
   close() {
+    return Promise.resolve();
   }
 }
 
@@ -443,7 +444,6 @@ class Publisher extends Source {
   }
 
   close() {
-    this.connection.removeMediaStream(this.mediaStream.id);
     this.connection.removeListener('status_event', this._connectionListener);
     if (this.mediaStream.monitorInterval) {
       clearInterval(this.mediaStream.monitorInterval);
@@ -453,6 +453,7 @@ class Publisher extends Source {
       clearInterval(this.mediaStream.periodicPlis);
       this.mediaStream.periodicPlis = undefined;
     }
+    return this.connection.removeMediaStream(this.mediaStream.id);
   }
 }
 
@@ -483,6 +484,7 @@ class ExternalInput extends Source {
   }
   // eslint-disable-next-line class-methods-use-this
   close() {
+    return Promise.resolve();
   }
 }
 

erizo_controller/erizoJS/models/Subscriber.js

@@ -127,16 +127,21 @@ class Subscriber extends NodeClass {
   }
 
   close() {
-    log.debug(`msg: Closing subscriber, streamId:${this.streamId}`);
-    this.publisher = undefined;
-    if (this.connection) {
-      this.connection.removeMediaStream(this.mediaStream.id);
-      this.connection.removeListener('status_event', this._connectionListener);
-      this.connection.removeListener('media_stream_event', this._mediaStreamListener);
-    }
-    if (this.mediaStream && this.mediaStream.monitorInterval) {
-      clearInterval(this.mediaStream.monitorInterval);
-    }
+    return new Promise((resolve) => {
+      log.debug(`msg: Closing subscriber, streamId:${this.streamId}`);
+      this.publisher = undefined;
+      if (this.mediaStream && this.mediaStream.monitorInterval) {
+        clearInterval(this.mediaStream.monitorInterval);
+      }
+      if (this.connection) {
+        this.connection.removeMediaStream(this.mediaStream.id).then(() => {
+          this.connection.removeListener('status_event', this._connectionListener);
+          this.connection.removeListener('media_stream_event', this._mediaStreamListener);
+          resolve();
+        });
+      }
+      resolve();
+    });
   }
 }