Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make RTCDataChannels Transferable #317

Closed
wants to merge 3 commits into from
Closed

Make RTCDataChannels Transferable #317

wants to merge 3 commits into from

Conversation

jesup
Copy link

@jesup jesup commented Sep 25, 2015

Per discussion in the list and frequent requests, here's a PR for Transferable RTCDataChannels. This likely isn't even officially needed, as adding it to the WhatWG list is likely all that's needed to make it transferable, but we should have a section on Transferability and the implications for in-flight events for implementors.

@martinthomson
Copy link
Member

Transferable implies more than just being portable to Workers. It means that you can also send the data channel to another window.

<h3>RTCDataChannel and Workers</h3>
<p>An <code><a>RTCDataChannel</a></code> object MUST be
<code><a href="https://html.spec.whatwg.org/multipage/infrastructure.html#transferable-objects">
Transferable</a></code> to <code>Workers</code> [[!WEBWORKERS]]. When it's
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this require changes to WebIDL? (at least modifying the [Exposed] tag)

Copy link
Member

Choose a reason for hiding this comment

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

[Exposed] doesn't need to change here. We can expose RTCDataChannel in workers with no effort.

If the intent is to limit this to a worker (of what sort?!?), then we either have a mistake on the part of @jesup or maybe a misunderstanding about what transferable means.

My understanding is that a transferable object can be passed through postMessage to other origins as well as workers. We should simply identify that RTCDataChannel as [Transferable] and note the consequences of that.

Copy link
Contributor

Choose a reason for hiding this comment

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

The specification is confusing on this point:
http://www.w3.org/TR/html51/infrastructure.html#transferable-objects
"Some objects support being copied and closed in one operation. This is called transferring the object, and is used in particular to transfer ownership of unsharable or expensive resources across worker boundaries."

@annevk can you weigh in here?

@martinthomson
Copy link
Member

Göran Eriksson made a good point on the list. We need to ensure that the RTCDataChannel isn't transferred to a service worker. We don't know how to make web sockets work properly in a service worker; the same applies to RTCDataChannel.

@fluffy
Copy link
Contributor

fluffy commented Oct 30, 2015

EKR is going to do security review

@alvestrand alvestrand removed the LATER label Nov 12, 2015
@stefhak
Copy link
Contributor

stefhak commented Nov 25, 2015

@ekr could you review this one (as we decided in Sapporo)?

@stefhak
Copy link
Contributor

stefhak commented Dec 9, 2015

@ekr has been pinged, but no response.

@alvestrand
Copy link
Contributor

@jesup we're not getting any response on our review request. Can you prod?

@ekr
Copy link
Contributor

ekr commented Dec 24, 2015

@alvestrand not sure why you think having @jesup ping me is going to make me do this any faster.

Anyway, I've reviewed this and modulo the issue about cross-origin transfers, seems roughly sound. One thing that I would observe is that there has been a bunch of discussion at various times about the interaction of DataChannels with SOP and in particular that DataChannels provide a mechanism for directly moving data from origin A to origin B in a way that is immune to SOP as well as to mixed-content warnings (what I mean here is that while you can't XHR from https:// origins to http:// origins, you can perfectly well DataChannel between them if the server allows both HTTP and HTTPS).

If it's indeed possible to move one end of a DataChannel between origins using this technique, then that makes this sort of thing something that can happen solely in the browser: create a DataChannel in an http: origin and then post-message to a https: origin. This is something we try to stop when it's done through XHR. With that said, you could just as easily have the http origin proxy the JSEP for the https origin and achieve this effect, so it's not clear that this is making things worse.

@ekr
Copy link
Contributor

ekr commented Dec 26, 2015

I talked separately with @annevk and he says that you can indeed transfer Transferrable things across origins. As @martinthomson suggests, we should make this point clear in the text. It might also be worthwhile making clear the point I suggested above.

@annevk
Copy link
Member

annevk commented Jan 1, 2016

The change here does not seem to carefully define what transfering actually does and what happens to the original object and any future operations applied to the original object.

whatwg/html#444 is a bit more realistic changeset of turning an object into a transferable object.

@stefhak
Copy link
Contributor

stefhak commented Jan 7, 2016

@annevk seems that PR #445 is a newer version for this functionality (your comment is valid there as well though).

@stefhak
Copy link
Contributor

stefhak commented Jan 7, 2016

Closing since #445 is a newer version for this functionality.

@stefhak stefhak closed this Jan 7, 2016
@alvestrand
Copy link
Contributor

Reopening since it turns out the text changes in #317 and #445 don't actually overlap.

@stefhak
Copy link
Contributor

stefhak commented Jan 11, 2016

@martinthomson looking back at your Sept 28 comment: how to specify that a data channel can't be transferred to a Service Worker? How is that done for WebSocket?

@annevk
Copy link
Member

annevk commented Jan 11, 2016

The WebSocket API isn't transferable. First you'd need to make sure the IDL has something like [Exposed=(Window,DedicatedWorker,SharedWorker)]. Then I guess the transfer algorithm needs some kind of check around that and fail. @domenic, any ideas?

@domenic
Copy link
Contributor

domenic commented Jan 11, 2016

None in particular.

@annevk
Copy link
Member

annevk commented Jan 11, 2016

I guess the "structured clone" algorithm needs to be refactored to take into account input and output Realm. Then there needs to be some kind of lookup to see whether the object from the input Realm can be created in the output Realm (using the Exposed IDL stuff), and then that needs to be able to fail (presumably through a DataCloneError). Patches for https://github.com/whatwg/html appreciated.

@jesup
Copy link
Author

jesup commented Jan 11, 2016

Transferring a DataChannel to a ServiceWorker may not be very useful (due to lack of global state), but is it something that needs to specifically dis-allowed? That said, if it's not useful, perhaps it makes sense to disallow it. Open to whatever wording makes sense. (What does make sense to be 'transferable' to a ServiceWorker?)

@feross
Copy link

feross commented Jan 11, 2016

DataChannel would be very useful in ServiceWorker. Here are two use cases that this opens up:

  1. Decentralized web apps, i.e. intercepting http requests and handling them with P2P systems like WebTorrent or IPFS.

    I'm aware of several attempts to build decentralize app distribution DataChannel-based P2P systems. PeerCloud and Webtorrentapp are just two examples.

    You can see how the lack of ServiceWorkers impairs the user experience of PeerCloud by visiting the demo page: https://peercloud.io The site tells users: "You must leave this page open while browsing! This is because Chrome doesn't support WebRTC from service workers." PeerCloud had to resort to shuttling requests from the ServiceWorker back to the main page in order to use DataChannel.

    We can do better than this.

  2. Peer-assisted delivery, i.e. hybrid P2P-HTTP content delivery networks.

    There are lots of these services popping up and they help site owners to reduce bandwidth costs, and enable new types of apps by dramatically reducing delivery costs. See PeerCDN (acquired by Yahoo), StreamRoot (just raised $2.5M in funding), Greta, and Peer5.

@jbenet
Copy link

jbenet commented Jan 11, 2016

Author of IPFS here. I echo @feross's statement fully. We really need DataChannels in ServiceWorker to get a good perf, seamless experience. This is really important for our full implementation of the IPFS protocol in js with webrtc. (And we fit in both the use cases @feross mentions)

@hexonaut
Copy link

Agree with @feross . The use case of hybrid P2P-HTTP CDNs in particular has the potential to offload a significant amount of bandwidth from the server. Having a transparent, drop-in service with all the bells and whistles that Service Worker can provide is a big help for pushing this forward.

@transitive-bullshit
Copy link

👍 I think this use case will allow a lot of valuable unforeseen possibilities.

@jesup
Copy link
Author

jesup commented Jan 11, 2016

@feross While I'd love to make them usable from ServiceWorkers, there are practical and security/etc issues that make this hard, I believe (if anyone knows I'm wrong on these statements below, please correct me - I'm not an expert on Service Workers).

The biggest issue is that what you want is PeerConnections in ServiceWorkers, not really making DataChannels transferable to ServiceWorkers. Without PeerConnections in the ServiceWorker, the connection must die when the PeerConnection's page goes out of scope. Transferring a PeerConnection to a ServiceWorker adds a lot of real problematic issues to resolve, both technically and from a user-control/privacy/security/etc perspective. That would be a separate discussion (for next-gen).

So there's no advantage to just transferring a DataChannel to a ServiceWorker (instead of Worker). This is why I indicated it was "not useful", but I don't see any direct harm from it. (Martin?)

DataChannels in Workers - those are what this PR is about, and those are absolutely useful.

@ghost
Copy link

ghost commented Jan 11, 2016

This Q&A is useful: http://stackoverflow.com/questions/29741922/prevent-service-worker-from-automatically-stopping

Creating a Websocket (and by extension RTCPeerConnection) in a ServiceWorker is not useful because its lifecycle is tied to the fetch event. It will die usually immediately afterwards, before you can do anything useful with the socket.
SharedWorker on the other hand lives for as long as there's a single dependent context. SharedWorker is where you want to do background processing, resource sharing, and cache writing.

@martinthomson
Copy link
Member

@adria2 has the right answer here. WebSocket and DataChannel both have a usage model that is incompatible with the idea that a ServiceWorker runs for a limited time. Both represent a way to keep the SW alive permanently, undermining the basic principle that states that a SW is only activated in response to certain types of events.

Running these things in a SW also makes user accountability very difficult, if not impossible.

@Zorlin
Copy link

Zorlin commented Jan 12, 2016

+1 for @feross. All excellent points.

@PixelsCommander
Copy link

+1 for @feross request, ViralJS poc would be better with this https://github.com/PixelsCommander/ViralJS

@stefhak
Copy link
Contributor

stefhak commented Jan 12, 2016

It looks to me like there are some things to be done here:

Ideally this should be combined into one PR. Anyone willing to take on this work? @jesup or @ekr ?

@jesup
Copy link
Author

jesup commented Jan 12, 2016

@martinthomson Aha - so if I'm correct your concern is that by allowing it to be transferred to a ServiceWorker, it would provide a way to subvert the lifetime limitations of a ServiceWorker? If so (and if other equivalent ways to subvert it don't exist, so this would be a new thing), then I can see bothering to disallow it.

If it's not new to be able to extend the lifetime, I don't see the need to block it here.

@feross - I don't see any short-term possibility of getting PeerConnections in ServiceWorkers. Without that, I don't see how this can achieve what you want. Even long-term, there will be serious questions about how this could be abused and affect privacy and performance. In any case, that would be a separate PR.

@annevk
Copy link
Member

annevk commented Jan 13, 2016

I don't really follow the concerns about extending a service worker's lifetime. Service workers have waitUntil() available. There's numerous other tricks available as well depending on which user agent you are dealing with. User agents will have to limit the lifetime of a service worker, but that is rather API independent. And APIs, especially network APIs, will need to define their garbage collection semantics and potentially what happens if they get cleaned up aggressively. This is nothing new however and also applies in other environments.

@annevk
Copy link
Member

annevk commented Jan 13, 2016

(As for updating W3C HTML, I wouldn't bother. It's not maintained and no sane user agent implements from that.)

@pirate
Copy link

pirate commented Jan 14, 2016

+1 for DataChannels in ServiceWorkers, although I hope we can eventually get PeerConnections as well.

@stefhak
Copy link
Contributor

stefhak commented Jan 28, 2016

This PR (and #445) needs work. If no one does the work it can't be part of 1.0.

@alvestrand
Copy link
Contributor

Closing while preparing for 1.0. It can be reopened later.

@kumavis
Copy link

kumavis commented Apr 19, 2017

@alvestrand @jesup it seems 1.0 was achieved. very exciting!
Would now be an appropriate time to re-open this discussion?

@annevk
Copy link
Member

annevk commented Apr 19, 2017

If you start working on this again please loop @domenic and I in. There have been some changes (mostly for the better in terms of what you needed last time this was discussed).

@isaackwan
Copy link

Are there any technical limitations around exposing RTCDataChannel to Workers, before making them transferable from other worker scopes?

@benjamingr
Copy link

Are there any technical limitations around exposing RTCDataChannel to Workers, before making them transferable from other worker scopes?

No, it's just stalled, @feross opened an issue about it ~3 years ago.

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

Successfully merging this pull request may close these issues.

None yet