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

More Conservative RTCPeerConnection Instantiation #2591

Closed
justin0mcateer opened this issue Jun 14, 2024 · 0 comments · Fixed by #2593 or #2585
Closed

More Conservative RTCPeerConnection Instantiation #2591

justin0mcateer opened this issue Jun 14, 2024 · 0 comments · Fixed by #2593 or #2585
Labels
need/triage Needs initial labeling and prioritization

Comments

@justin0mcateer
Copy link

  • Version:
    libp2p: 1.2.4
    @libp2p/webrtc: 4.0.22

  • Platform:
    Linux master 6.8.6-arch1-1 #1 SMP PREEMPT_DYNAMIC Sat, 13 Apr 2024 14:42:24 +0000 x86_64 GNU/Linux

  • Subsystem:
    WebRTCTransport dial

Severity:

Medium

Description:

The 'dial' method in WebRTCTransport creates an RTCPeerConnection as it's first step. However, there are several dependencies to actually being able to negotiate the WebRTC connection, including being able to connect to the Circuit Relay. If the Circuit Relay is not reachable for any reason, a stream cannot be opened, or other possible issues, the dial fails and the RTCPeerConnection is wasted.

This seems like a nit, but it creates a significant issue because:

  1. Dialing is often done automatically on a periodic basis
  2. Chrome has a hard limit of 500 RTCPeerConnections instantiated in the lifetime of a Chrome process (browser tab).
    https://issues.chromium.org/issues/41378764

For long-running libp2p instances, this can easily present an issue. Once the issue occurs, the only recourse is the close the tab, even refreshing will not work.

Steps to reproduce the error:

  • Attempt to dial a WebRTC peer address 510 times via an unreachable Circuit Relay address
  • Observe the dial attempts will fail forever after the 500th attempt on the RTCPeerConnection instantiation
  • Observe that even if the Circuit Relay becomes available after 500 failures, a WebRTC connection will never be established again

Perhaps the RTCPeerConnection construction could be moved inside of 'initiateConnection' and returned from that function?
https://github.com/libp2p/js-libp2p/blob/main/packages/transport-webrtc/src/private-to-private/initiate-connection.ts#L29

This would allow the address parsing, relay connection and stream establishment to occur before the RTCPeerConnection construction. That way, if any of the pre-requisite steps fail, the RTCPeerConnection is not 'wasted'.

Granted, this is only an issue because of bad behavior in Chrome. However, this has been a known issue for several years and no progress has been made in fixing it. The affects all Chrome derivatives (Edge, Brave, etc), which taken together represent the vast majority of the browser market.

@justin0mcateer justin0mcateer added the need/triage Needs initial labeling and prioritization label Jun 14, 2024
achingbrain added a commit that referenced this issue Jun 17, 2024
Chrome limits how many RTCPeerConnections a given tab can instantiated
during it's lifetime - https://issues.chromium.org/issues/41378764

To delay hitting this limit, only create the dial-end RTCPeerConnection
once a relayed connection has successfully been opened to the dial
target, this prevents needlessly creating RTCPeerConnections when the
dial fails before they are actually used.

Fixes #2591
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/triage Needs initial labeling and prioritization
Projects
None yet
1 participant