Skip to content

Commit

Permalink
fix(JitsiConference): Clear jingleSession after session restart.
Browse files Browse the repository at this point in the history
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 jitsi/jitsi-meet#14326.
  • Loading branch information
jallamsetty1 authored and subhamcyara committed Jul 19, 2024
1 parent f3e7af0 commit 4ee3ac6
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 33 deletions.
71 changes: 59 additions & 12 deletions JitsiConference.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
24 changes: 6 additions & 18 deletions modules/connectivity/IceFailedHandling.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
10 changes: 7 additions & 3 deletions modules/connectivity/IceFailedHandling.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ class MockConference extends Listenable {
config: { }
};
}

/**
* Mock function.
*/
_stopJvbSession() {} // eslint-disable-line no-empty-function
}

describe('IceFailedHandling', () => {
Expand Down Expand Up @@ -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();
Expand All @@ -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,
Expand Down

0 comments on commit 4ee3ac6

Please sign in to comment.