From 4ee3ac6b7afa41e7f856de0c3be30eaa5e866ac7 Mon Sep 17 00:00:00 2001 From: Jaya Allamsetty Date: Thu, 15 Feb 2024 14:18:16 -0500 Subject: [PATCH] fix(JitsiConference): Clear jingleSession after session restart. This makes sure that any track operations that are executed after the terminate is sent and before the new session is established get synced up on the new session. Also reset the JVB stats. Fixes https://github.com/jitsi/jitsi-meet/issues/14326. --- JitsiConference.js | 71 +++++++++++++++---- modules/connectivity/IceFailedHandling.js | 24 ++----- .../connectivity/IceFailedHandling.spec.js | 10 ++- 3 files changed, 72 insertions(+), 33 deletions(-) diff --git a/JitsiConference.js b/JitsiConference.js index 75e9c7b084..11bf18793e 100644 --- a/JitsiConference.js +++ b/JitsiConference.js @@ -3367,13 +3367,63 @@ JitsiConference.prototype._shouldBeInP2PMode = function() { return shouldBeInP2P; }; +/** + * Stops the current JVB jingle session. + * + * @param {Object} options - options for stopping JVB session. + * @param {string} options.reason - One of the Jingle "reason" element + * names as defined by https://xmpp.org/extensions/xep-0166.html#def-reason + * @param {string} options.reasonDescription - Text description that will be included in the session terminate message. + * @param {boolean} options.requestRestart - Whether this is due to a session restart, in which case, session will be + * set to null. + * @param {boolean} options.sendSessionTerminate - Whether session-terminate needs to be sent to Jicofo. + */ +JitsiConference.prototype._stopJvbSession = function(options = {}) { + const { + requestRestart = false, + sendSessionTerminate = false + } = options; + + if (!this.jvbJingleSession) { + logger.error('No JVB session to be stopped'); + + return; + } + + // Remove remote JVB tracks. + !this.isP2PActive() && this._removeRemoteJVBTracks(); + + logger.info('Stopping stats for jvb connection'); + this.statistics.stopRemoteStats(this.jvbJingleSession.peerconnection); + + this.jvbJingleSession.terminate( + () => { + if (requestRestart && sendSessionTerminate) { + logger.info('session-terminate for ice restart - done'); + } + this.jvbJingleSession = null; + }, + error => { + if (requestRestart && sendSessionTerminate) { + logger.error('session-terminate for ice restart failed: reloading the client'); + + // Initiate a client reload if Jicofo responds to the session-terminate with an error. + this.eventEmitter.emit( + JitsiConferenceEvents.CONFERENCE_FAILED, + JitsiConferenceErrors.ICE_FAILED); + } + logger.error(`An error occurred while trying to terminate the JVB session', reason=${error.reason},` + + `msg=${error.msg}`); + }, + options); +}; + /** * Stops the current P2P session. * @param {Object} options - Options for stopping P2P. * @param {string} options.reason - One of the Jingle "reason" element * names as defined by https://xmpp.org/extensions/xep-0166.html#def-reason - * @param {string} options.reasonDescription - Text - * description that will be included in the session terminate message + * @param {string} options.reasonDescription - Text description that will be included in the session terminate message. * @param {boolean} requestRestart - Whether this is due to a session restart, in which case * media will not be resumed on the JVB. * @private @@ -3410,6 +3460,7 @@ JitsiConference.prototype._stopP2PSession = function(options = {}) { this.p2pJingleSession.terminate( () => { logger.info('P2P session terminate RESULT'); + this.p2pJingleSession = null; }, error => { // Because both initiator and responder are simultaneously @@ -3730,16 +3781,12 @@ JitsiConference.prototype._restartMediaSessions = function() { } if (this.jvbJingleSession) { - this.jvbJingleSession.terminate( - null /* success callback => we don't care */, - error => { - logger.warn('An error occurred while trying to terminate the JVB session', error); - }, { - reason: 'success', - reasonDescription: 'restart required', - requestRestart: true, - sendSessionTerminate: true - }); + this._stopJvbSession({ + reason: 'success', + reasonDescription: 'restart required', + requestRestart: true, + sendSessionTerminate: true + }); } this._maybeStartOrStopP2P(false); diff --git a/modules/connectivity/IceFailedHandling.js b/modules/connectivity/IceFailedHandling.js index f5381bbda1..666a0b3b7e 100644 --- a/modules/connectivity/IceFailedHandling.js +++ b/modules/connectivity/IceFailedHandling.js @@ -69,24 +69,12 @@ export default class IceFailedHandling { + `ICE state: ${jvbConnIceState}, ` + `use 'session-terminate': ${useTerminateForRestart}`); if (useTerminateForRestart) { - this._conference.jvbJingleSession.terminate( - () => { - logger.info('session-terminate for ice restart - done'); - }, - error => { - logger.error(`session-terminate for ice restart failed: reason=${error.reason},` - + `message=${error.msg}`); - - // Initiate a client reload if Jicofo responds to the session-terminate with an error. - this._conference.eventEmitter.emit( - JitsiConferenceEvents.CONFERENCE_FAILED, - JitsiConferenceErrors.ICE_FAILED); - }, { - reason: 'connectivity-error', - reasonDescription: 'ICE FAILED', - requestRestart: true, - sendSessionTerminate: true - }); + this._conference._stopJvbSession({ + reason: 'connectivity-error', + reasonDescription: 'ICE FAILED', + requestRestart: true, + sendSessionTerminate: true + }); } else { this._conference.jvbJingleSession.sendIceFailedNotification(); } diff --git a/modules/connectivity/IceFailedHandling.spec.js b/modules/connectivity/IceFailedHandling.spec.js index 26f577bb8f..e46fd5266d 100644 --- a/modules/connectivity/IceFailedHandling.spec.js +++ b/modules/connectivity/IceFailedHandling.spec.js @@ -16,6 +16,11 @@ class MockConference extends Listenable { config: { } }; } + + /** + * Mock function. + */ + _stopJvbSession() {} // eslint-disable-line no-empty-function } describe('IceFailedHandling', () => { @@ -126,7 +131,7 @@ describe('IceFailedHandling', () => { // eslint-disable-next-line no-empty-function terminate: () => { } }; - sendSessionTerminateSpy = spyOn(mockConference.jvbJingleSession, 'terminate'); + sendSessionTerminateSpy = spyOn(mockConference, '_stopJvbSession'); }); it('send "session-terminate" with the request restart attribute', () => { iceFailedHandling.start(); @@ -135,8 +140,7 @@ describe('IceFailedHandling', () => { .then(() => nextTick(2500)) // tick for ice timeout .then(() => { expect(sendSessionTerminateSpy).toHaveBeenCalledWith( - jasmine.any(Function), - jasmine.any(Function), { + { reason: 'connectivity-error', reasonDescription: 'ICE FAILED', requestRestart: true,