Skip to content

Commit

Permalink
Fix use after destroy().
Browse files Browse the repository at this point in the history
This is a partial cherry-pick of
f48f7fd.

Issue #1323
Issue #1423
  • Loading branch information
TheModMaker committed May 15, 2018
1 parent 253eaf7 commit 5ccdd31
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 6 deletions.
10 changes: 10 additions & 0 deletions lib/cast/cast_proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,11 @@ shaka.cast.CastProxy.prototype.cast = function() {
// TODO: transfer side-loaded text tracks?

return this.sender_.cast(initState).then(function() {
if (!this.localPlayer_) {
// We've already been destroyed.
return;
}

// Unload the local manifest when casting succeeds.
return this.localPlayer_.unload();
}.bind(this));
Expand Down Expand Up @@ -391,6 +396,11 @@ shaka.cast.CastProxy.prototype.onResumeLocal_ = function() {

// Finally, take on video state and player's "after load" state.
manifestReady.then(function() {
if (!this.localVideo_) {
// We've already been destroyed.
return;
}

shaka.cast.CastUtils.VideoInitStateAttributes.forEach(function(name) {
this.localVideo_[name] = videoState[name];
}.bind(this));
Expand Down
15 changes: 15 additions & 0 deletions lib/cast/cast_receiver.js
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,11 @@ shaka.cast.CastReceiver.prototype.onGenericMessage_ = function(event) {
}
case 'STOP':
this.player_.unload().then(function() {
if (!this.player_) {
// We've already been destroyed.
return;
}

this.sendMediaStatus_(0);
}.bind(this));
break;
Expand Down Expand Up @@ -653,6 +658,11 @@ shaka.cast.CastReceiver.prototype.onGenericMessage_ = function(event) {
if (autoplay)
this.video_.autoplay = true;
this.player_.load(manifestUri, currentTime).then(function() {
if (!this.player_) {
// We've already been destroyed.
return;
}

// Notify generic controllers that the media has changed.
this.sendMediaInfoMessage_();
}.bind(this)).catch(function(error) {
Expand Down Expand Up @@ -694,6 +704,11 @@ shaka.cast.CastReceiver.prototype.onGenericMessage_ = function(event) {
*/
shaka.cast.CastReceiver.prototype.sendAsyncComplete_ =
function(senderId, id, error) {
if (!this.player_) {
// We've already been destroyed.
return;
}

this.sendMessage_({
'type': 'asyncComplete',
'id': id,
Expand Down
2 changes: 1 addition & 1 deletion lib/media/drm_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -1350,7 +1350,7 @@ shaka.media.DrmEngine.prototype.onKeyStatusesChange_ = function(event) {
if (!this.activeSessions_[i].updatePromise) {
shaka.log.debug('Session has expired', session);
this.activeSessions_.splice(i, 1);
session.close();
session.close().catch(function() {}); // Silence uncaught reject errors
}
}

Expand Down
16 changes: 16 additions & 0 deletions lib/media/streaming_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,10 @@ shaka.media.StreamingEngine.prototype.init = function() {
// Setup the initial set of Streams and then begin each update cycle. After
// startup completes onUpdate_() will set up the remaining Periods.
return this.initStreams_(initialStreams).then(function() {
if (this.destroyed_) {
return;
}

shaka.log.debug('init: completed initial Stream setup');

// Subtlety: onInitialStreamsSetup() may call switch() or seeked(), so we
Expand Down Expand Up @@ -1100,6 +1104,10 @@ shaka.media.StreamingEngine.prototype.onUpdate_ = function(mediaState) {
if (mediaStates.every(function(ms) { return ms.endOfStream; })) {
shaka.log.v1(logPrefix, 'calling endOfStream()...');
this.playerInterface_.mediaSourceEngine.endOfStream().then(function() {
if (this.destroyed_) {
return;
}

// If the media segments don't reach the end, then we need to update the
// timeline duration to match the final media duration to avoid buffering
// forever at the end. We should only do this if the duration needs to
Expand Down Expand Up @@ -1885,6 +1893,10 @@ shaka.media.StreamingEngine.prototype.handleStartup_ = function(
// Period is probably the initial one.
if (!this.canSwitchPeriod_[currentPeriodIndex]) {
this.setupPeriod_(currentPeriodIndex).then(function() {
if (this.destroyed_) {
return;
}

shaka.log.v1(logPrefix, 'calling onCanSwitch()...');
this.playerInterface_.onCanSwitch();
}.bind(this)).catch(Functional.noop);
Expand Down Expand Up @@ -2201,6 +2213,10 @@ shaka.media.StreamingEngine.prototype.handleStreamingError_ = function(error) {
// rapid retry cycle that could be very unkind to the server. Instead,
// use the backoff system to delay and backoff the error handling.
this.failureCallbackBackoff_.attempt().then(function() {
if (this.destroyed_) {
return;
}

// First fire an error event.
this.playerInterface_.onError(error);

Expand Down
10 changes: 8 additions & 2 deletions test/media/drm_engine_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -1484,8 +1484,14 @@ describe('DrmEngine', function() {
initAndAttach().then(function() {
session1.closed = new shaka.util.PublicPromise();
session2.closed = new shaka.util.PublicPromise();
session1.close.and.returnValue(Promise.reject());
session2.close.and.returnValue(Promise.reject());

// Since this won't be attached to anything until much later, we must
// silence unhandled rejection errors.
const rejected = Promise.reject();
rejected.catch(function() {});

session1.close.and.returnValue(rejected);
session2.close.and.returnValue(rejected);

var initData1 = new Uint8Array(1);
var initData2 = new Uint8Array(2);
Expand Down
3 changes: 2 additions & 1 deletion test/player_integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,8 @@ describe('Player', function() {
function testTemplate(operationFn) {
// No data will be loaded for this test, so it can use a real manifest
// parser safely.
player.load('reject://www.foo.com/bar.mpd').then(fail);
player.load('reject://www.foo.com/bar.mpd')
.then(fail).catch(function() {});
return shaka.test.Util.delay(0.1).then(operationFn).then(function() {
expect(schemeSpy.calls.count()).toBe(1);
});
Expand Down
2 changes: 1 addition & 1 deletion test/player_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ describe('Player', function() {
});
var factory = function() { return parser; };

player.load('', 0, factory).then(fail);
player.load('', 0, factory).then(fail).catch(function() {});
shaka.test.Util.delay(0.1).then(function() {
player.destroy().catch(fail).then(function() {
expect(abrManager.stop).toHaveBeenCalled();
Expand Down
2 changes: 1 addition & 1 deletion test/util/cancelable_chain_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ describe('CancelableChain', function() {
var wait = shaka.test.Util.delay(1.0);
chain.cancel(cannedError);
return wait;
}).finalize().then(fail);
}).finalize().then(fail).catch(function() {});
});
});
});

0 comments on commit 5ccdd31

Please sign in to comment.