Bug 173052 - RTCDataChannel connectivity issues in Safari 11
Summary: RTCDataChannel connectivity issues in Safari 11
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-06-07 03:54 PDT by Ashley Gullen
Modified: 2017-09-14 12:45 PDT (History)
9 users (show)

See Also:


Attachments
Patch (7.47 KB, patch)
2017-09-14 10:06 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (7.70 KB, patch)
2017-09-14 11:26 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ashley Gullen 2017-06-07 03:54:00 PDT
WebRTC DataChannels appear to have connectivity issues on Safari 11 with macOS High Sierra.

This is a simple DataChannel-based multiplayer game demo made in Construct 2: https://www.scirra.com/labs/rtcgame/

Simply open the page, type in a name and click 'Join'. If you do this with two browsers, you should be able to see each player's actions as they move around with WASD and mouse move & click. However in Safari 11 the following cases fail to connect where Chrome can:

1) Two Safari windows on the same machine cannot connect
2) Safari & Chrome on a Windows 10 machine on the same LAN cannot connect
3) Safari & Chrome on a remote Windows machine (over the Internet) establish a connection, but after a moment the DataChannel send() method starts throwing "InvalidStateError (DOM Exception 11): The object is in an invalid state."

Chrome and Firefox can interoperate in all of these cases; so should Safari.
Comment 1 youenn fablet 2017-06-08 08:41:08 PDT
(In reply to Ashley Gullen from comment #0)
> WebRTC DataChannels appear to have connectivity issues on Safari 11 with
> macOS High Sierra.
> 
> This is a simple DataChannel-based multiplayer game demo made in Construct
> 2: https://www.scirra.com/labs/rtcgame/
> 
> Simply open the page, type in a name and click 'Join'. If you do this with
> two browsers, you should be able to see each player's actions as they move
> around with WASD and mouse move & click. However in Safari 11 the following
> cases fail to connect where Chrome can:
> 
> 1) Two Safari windows on the same machine cannot connect
> 2) Safari & Chrome on a Windows 10 machine on the same LAN cannot connect
> 3) Safari & Chrome on a remote Windows machine (over the Internet) establish
> a connection, but after a moment the DataChannel send() method starts
> throwing "InvalidStateError (DOM Exception 11): The object is in an invalid
> state."
> 
> Chrome and Firefox can interoperate in all of these cases; so should Safari.

Hi Ashley, thanks for the report.
Contrary to Chrome and Firefox, we do not leak private IP addresses access unless camera or microphone access is also granted.

You can use the Debug menu (see https://webkit.org/debugging-webkit/ to use "defaults write" magic to make it appear). Then you can go to "Media Flags" and check "Disable ICE candidate filtering". Once these options are set, make sure to create a new tab so that these options are propagated properly.

https://webrtc.github.io/samples/src/content/datachannel/basic/ does not work without this change but works with this change.

Would be great if you can test that and report the results.
Comment 2 Ashley Gullen 2017-06-08 09:37:09 PDT
Hi youenn,

I must emphasise case #3 was not a local case so I do not expect your suggestion to change it. However I have checked "Disable ICE candidate filtering" and restarted Safari and tested each case again. Results are:

1) Two Safari windows on the same machine: a connection is established, but send() calls fail with "InvalidStateError (DOM Exception 11): The object is in an invalid state."
2) Safari & Chrome on LAN: it works for a couple of seconds (players can see each other moving), then after a few seconds send() fails with "InvalidStateError (DOM Exception 11): The object is in an invalid state."
3) Safari & Chrome over Internet: identical to previous result (as expected, since not a local connection)

So there appear to be two issues here:
A) Local connections are blocked by default (apparently intentionally)
B) InvalidStateError occurs very soon after establishing a DataChannel connection

B definitely needs to be fixed, but I must try to make a case about A. Multiplayer games are a good use case for DataChannels, and players will frequently want to play over a LAN. Many games have no use for camera or microphone. It also encourages a weird workaround where games would have to ask for media permissions they don't need just to gain LAN connection permission. Alternatively we can put TURN servers in place, but this makes WebRTC games much more expensive to operate as you then have to run the relay server and pay its bandwidth bills, *and* the network performance will be far worse than the low-latency, high-bandwidth local connection that could otherwise be made. In practice this will result in games that are laggy in Safari and much more responsive in Chrome or Firefox.

There are also non-game use cases for local DataChannels: we have a browser-based game development IDE at https://editor.construct.net. It has a "remote preview" feature that allows you to preview your game on another device. It works by sending the entire game down a DataChannel to another device for testing, e.g. your phone. Many of our users are keen for this to work on iOS. In this case we run a TURN server. If a user has a 100mb game and wants to run it on their iPhone, in this case they'll have to wait for a huge transfer over the Internet (while running up our bandwidth bills) just to transfer it across their desk to their phone. It runs at LAN speed in Chrome, so again this will mean that fast, snappy loads in Chrome for Android are a slow drag on Safari.

I am sure there are several other use cases for DataChannels over LAN.

Besides, what's the concern about leaking a private IP? For a nontrivial percentage of users it can be guessed, e.g. it's probably 192.168.0.1 for a lot of people. I'd be interested to know the rationale behind this. Perhaps there's something else that can be done? Surely online games don't have to suffer this?
Comment 3 youenn fablet 2017-06-08 11:34:05 PDT
One case where this error is thrown is when the channel is not yet ready (connection not opened...), as per https://w3c.github.io/webrtc-pc/#dfn-send.

Maybe there is a difference in behavior with other browsers with that regards.
Maybe send() is called to soon in your app, or maybe we have a bug on our side.

I'll try to have a closer look at this issue, probably by end of next week.
It would help if you can provide a reduced test case.
Comment 4 Ashley Gullen 2017-06-12 07:18:00 PDT
The code in the demo specifically waits until the DataChannel onopen handler fires before trying to call send(). It's possible there's a bug in that code, but I suspect not based on the fact in some cases it works briefly, with data correctly being received, before failing with InvalidStateError.
Comment 5 Radar WebKit Bug Importer 2017-06-12 07:18:19 PDT
<rdar://problem/32712143>
Comment 6 Jon Lee 2017-06-13 19:07:23 PDT
*** Bug 173307 has been marked as a duplicate of this bug. ***
Comment 7 youenn fablet 2017-09-13 21:59:34 PDT
I looked at rtcgame more closely.
It seems that a lot of big messages (>200Ko) are tried to be sent through RTCDataChannel, apparently more than the bandwidth available.
If more data is sent, it is buffered up to 16Mo when the channel is closed, hence the InvalidStateError.
Do you know what kind of data is sent and why it would be so big? 

Looking at Chrome, it does not appear that a lot of data is sent.
Comment 8 Ashley Gullen 2017-09-14 04:11:03 PDT
That sounds wrong, it should only be sending a tiny amount of data for player position and state - definitely <5kb per message.

It works correctly in Chrome and Firefox so maybe there's an issue with DataChannels or perhaps there's some compatibility issue with Safari in the code that generates the messages to send?
Comment 9 Ashley Gullen 2017-09-14 04:18:15 PDT
Oh, I just realised - our code reserves a 256kb ArrayBuffer and writes binary messages to be sent to that buffer. We then send a fraction of the buffer by creating a view to pass to send(), e.g.: new Uint8Array(buffer, 0, len)

I suspect in the ArrayBufferView overload of send(), Safari actually sends the entire ArrayBuffer. It should instead only send the data in the view.
Comment 10 Lennart Grahl 2017-09-14 05:34:38 PDT
Sorry for jumping in here but I have a few questions to youenn fablet's response from comment #7:

> It seems that a lot of big messages (>200Ko) are tried to be sent through
> RTCDataChannel, apparently more than the bandwidth available.

I don't understand the correlation between available bandwidth and being able to send a message of size n? Can you elaborate?

> If more data is sent, it is buffered up to 16Mo when the channel is closed,
> hence the InvalidStateError.

Why would the implementation buffer data on a non-open channel?
Comment 11 youenn fablet 2017-09-14 09:11:48 PDT
(In reply to Ashley Gullen from comment #9)
> Oh, I just realised - our code reserves a 256kb ArrayBuffer and writes
> binary messages to be sent to that buffer. We then send a fraction of the
> buffer by creating a view to pass to send(), e.g.: new Uint8Array(buffer, 0,
> len)
> 
> I suspect in the ArrayBufferView overload of send(), Safari actually sends
> the entire ArrayBuffer. It should instead only send the data in the view.

Thanks this is really useful!
There is probably a bug in our handling of ArrayBufferView here, I'll try to fix it asap.
Comment 12 youenn fablet 2017-09-14 09:14:14 PDT
(In reply to Lennart Grahl from comment #10)
> Sorry for jumping in here but I have a few questions to youenn fablet's
> response from comment #7:
> 
> > It seems that a lot of big messages (>200Ko) are tried to be sent through
> > RTCDataChannel, apparently more than the bandwidth available.
> 
> I don't understand the correlation between available bandwidth and being
> able to send a message of size n? Can you elaborate?

If the web application sends more data than what the channel can actually send, this data must be buffered by the web engine.

RTCDataChannel.bufferedAmount should give this information.

> > If more data is sent, it is buffered up to 16Mo when the channel is closed,
> > hence the InvalidStateError.
> 
> Why would the implementation buffer data on a non-open channel?

If the channel is not opened, an exception will happen and nothing will be buffered.
In the rtcgame case, the channel is opened and instead of sending a few bytes, we are probably wrongly sending 256 kb, quickly exploding the buffer.
Comment 13 youenn fablet 2017-09-14 10:06:33 PDT
Created attachment 320779 [details]
Patch
Comment 14 youenn fablet 2017-09-14 10:13:38 PDT
Patch seems to solve the rtcgame issue.
As of the ice candidate filtering issue, it might be best to file another bug dedicated to that specific topic.
Comment 15 Ashley Gullen 2017-09-14 10:25:22 PDT
That's great! Do you have any idea which Safari release or TP the fix will be in so I can verify?

I'll file a separate bug about local IP addresses.
Comment 16 Ashley Gullen 2017-09-14 10:46:30 PDT
Filed issue 176921 for making LAN connections.
Comment 17 Sam Weinig 2017-09-14 11:16:19 PDT
Comment on attachment 320779 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=320779&action=review

> Source/WebCore/ChangeLog:10
> +
> +        Covered by updated test.
> +

This needs an explanation of the change.
Comment 18 youenn fablet 2017-09-14 11:26:50 PDT
Created attachment 320794 [details]
Patch
Comment 19 youenn fablet 2017-09-14 11:27:43 PDT
(In reply to Sam Weinig from comment #17)
> Comment on attachment 320779 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=320779&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +
> > +        Covered by updated test.
> > +
> 
> This needs an explanation of the change.

Maybe we should have change the bug title.
Anyway, I beefed up the change log.
Comment 20 WebKit Commit Bot 2017-09-14 12:45:35 PDT
Comment on attachment 320794 [details]
Patch

Clearing flags on attachment: 320794

Committed r222045: <http://trac.webkit.org/changeset/222045>
Comment 21 WebKit Commit Bot 2017-09-14 12:45:37 PDT
All reviewed patches have been landed.  Closing bug.