1729405 - RTCPeerConnection.close throws in WPT

Status: RESOLVED FIXED

(Firefox :: Untriaged, defect, P2)

Description

[Philipp Hancke [:fippo]]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:91.0) Gecko/20100101 Firefox/91.0

Steps to reproduce:

https://github.com/web-platform-tests/wpt/pull/30213
was flaky in Firefox. I did some investigation in
https://bugs.chromium.org/p/webrtc/issues/detail?id=13148
and it looks like RTCPeerConnection.close() is throwing an error because its implementation is gone.

Expected results:

https://searchfox.org/mozilla-central/source/dom/media/PeerConnection.jsm#1529
should probably catch this error and ignore it.

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Thanks Philipp for filing! I'm able to repro with http://jsfiddle.net/jib1/u5tmrwf6/

I've located what I think is the problem: pc.close() deletes this._pc here, and SRD is referencing its glorified getter this._impl. in a few places asynchronously.

There are 41 hits for this._impl. and they should all be protected by checks against "closed" state. A few appear missing, e.g. restartIce (spec looks fine there though).

Comment 2

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Bug 1729405 - Test that SRD() and restartIce() don't error with pc.close(). r?bwc

Comment 3

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Bug 1729405 - Make SRD() and restartIce() not error on pc.close(). r?bwc

Depends on D124842

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b50031080353
Test that SRD() and restartIce() don't error with pc.close(). r=bwc
https://hg.mozilla.org/integration/autoland/rev/257ce52af62b
Make SRD() and restartIce() not error on pc.close(). r=bwc

Comment 5

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/b50031080353
https://hg.mozilla.org/mozilla-central/rev/257ce52af62b

Comment 6

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/30885 for changes under testing/web-platform/tests

Comment 7

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot