Skip to content

Prevent GC of non-closed RTCDataChannels - #2902

Merged
alvestrand merged 5 commits into
w3c:mainfrom
jan-ivar:datachannels
Nov 2, 2023
Merged

Prevent GC of non-closed RTCDataChannels#2902
alvestrand merged 5 commits into
w3c:mainfrom
jan-ivar:datachannels

Conversation

@jan-ivar

@jan-ivar jan-ivar commented Oct 6, 2023

Copy link
Copy Markdown
Member

Fixes #2624.


Preview | Diff

@jan-ivar jan-ivar self-assigned this Oct 6, 2023
@jan-ivar

Copy link
Copy Markdown
Member Author

Test forthcoming in https://phabricator.services.mozilla.com/D190744.

Comment thread webrtc.html
Comment thread amendments.json
Comment thread webrtc.html Outdated
</li>
<li>
<p>
Remove <var>channel</var> from <var>connection</var>.{{RTCPeerConnection/[[DataChannels]]}}.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems good to do it there so that calling close() will remove synchronously the dc from the set.
What about the case of a transport that gets errored?
Shouldn't we also do the same in https://w3c.github.io/webrtc-pc/#announcing-a-data-channel-as-closed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that "procedure" is also an algorithm called from https://w3c.github.io/webrtc-pc/#data-transport-closing-procedure

There's also this note suggesting the closing procedure is called from somewhere other than close() but it's not.
image

@alvestrand alvestrand left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only nit here is that "Ordered set" sounds prescriptive of implementation while seeming to have no JS-visible effect. I like minimally-constraining descriptions.

Comment thread webrtc.html
Comment thread webrtc.html
@alvestrand
alvestrand merged commit 813e08c into w3c:main Nov 2, 2023
@jan-ivar
jan-ivar deleted the datachannels branch November 2, 2023 15:45
@jan-ivar

Copy link
Copy Markdown
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should garbage collecting RTCDataChannels be observable?

6 participants