2007160 - Support Draining promise for WebTransport

Status: RESOLVED FIXED

(Core :: Networking, enhancement, P2)

Description

[Randell Jesup [:jesup] (needinfo me)]

Support the new Draining promise in the spec.
3.1:

A WebTransport session session is draining when the CONNECT stream is asked to gracefully close by the server, as described in [WEB-TRANSPORT-OVERVIEW] Section 4.1.

One part is handling lifetimes:

6.8. Garbage Collection

A WebTransport object whose [[State]] is "connecting" must not be garbage collected if [[IncomingBidirectionalStreams]], [[IncomingUnidirectionalStreams]], any WebTransportReceiveStream, or [[Datagrams]].[[Readable]] are locked, or if the ready, draining, or closed promise is being observed.

A WebTransport object whose [[State]] is "connected" must not be garbage collected if [[IncomingBidirectionalStreams]], [[IncomingUnidirectionalStreams]], any WebTransportReceiveStream, or [[Datagrams]].[[Readable]] are locked, or if the draining or closed promise is being observed.

A WebTransport object whose [[State]] is "draining" must not be garbage collected if [[IncomingBidirectionalStreams]], [[IncomingUnidirectionalStreams]], any WebTransportReceiveStream, or [[Datagrams]].[[Readable]] are locked, or if the closed promise is being observed.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Bug 2007160: Add draining promise attribute to WebTranport r=#necko-reviewers!,#dom-core!

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Bug 2007160: Update WebTransport getStats tests to include all fields r=jib!

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Bug 2007160: Add WPT test for WebTransport draining promise r=jib!

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Bug 2007160: Finish hooking up Draining from neqo for WebTransport. Add InvalidState error r=#necko-reviewers!

Comment 5

[Phabricator Automation]

Comment on attachment 9539722 [details]
Bug 2007160: Update WebTransport getStats tests to include all fields r=jib!

Revision D280255 was moved to bug 2007202. Setting attachment 9539722 [details] to obsolete.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e336c3b62c71
https://hg.mozilla.org/integration/autoland/rev/d5bab38fbfc7
Add draining promise attribute to WebTranport r=necko-reviewers,webidl,saschanaz,kershaw
https://github.com/mozilla-firefox/firefox/commit/15ec5c3090ee
https://hg.mozilla.org/integration/autoland/rev/fc3031b293b9
Finish hooking up Draining from neqo for WebTransport.  Add InvalidState error r=necko-reviewers,kershaw
https://github.com/mozilla-firefox/firefox/commit/bcd7d64215eb
https://hg.mozilla.org/integration/autoland/rev/289362b5613a
Add WPT test for WebTransport draining promise r=jib

Comment 7

[rperta]

Backed out for causing build bustages
Backout link
Push with failures
Failure log(s)

Comment 8

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

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

Comment 9

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

Upstream PR merged by moz-wptsync-bot

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/c24c6397a056
https://hg.mozilla.org/integration/autoland/rev/1673048dc8a8
Add draining promise attribute to WebTranport r=necko-reviewers,webidl,saschanaz,kershaw
https://github.com/mozilla-firefox/firefox/commit/ad4bbbb16a78
https://hg.mozilla.org/integration/autoland/rev/84a190af0b88
Finish hooking up Draining from neqo for WebTransport.  Add InvalidState error r=necko-reviewers,kershaw
https://github.com/mozilla-firefox/firefox/commit/a2b63643d80b
https://hg.mozilla.org/integration/autoland/rev/44bc740ec952
Add WPT test for WebTransport draining promise r=jib

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/1673048dc8a8
https://hg.mozilla.org/mozilla-central/rev/84a190af0b88
https://hg.mozilla.org/mozilla-central/rev/44bc740ec952

Comment 12

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Bug 2007160: fix race between OnDraining and session-ready retargeting r=#necko-reviewers!

WebTransportSessionProxy::OnDraining() consulted mTarget directly, unlike
its siblings (OnSessionClosed, OnAtomicWriteResult, OnDatagramReceivedInternal,
OnMaxDatagramSizeInternal), which all check mStopRequestCalled first. Since
the listener's OnSessionReady runs on the main thread and retargets mTarget
to the socket thread from there, a Draining event that arrives before that
retargeting completes sees the stale main-thread default and misdelivers
the listener call to the main thread instead of the socket thread. For
WebTransportParent, whose methods assert socket-thread affinity, this sends
the PWebTransport IPC message off its bound thread and crashes - observable
whenever a server drains a session immediately upon connecting. Queue the
event like the other listener callbacks do until OnStopRequest has run.

Comment 13

[BugBot [:suhaib / :marco]]

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)