search

UWNetworksLab / uProxy-p2p

Internet without borders
https://www.uproxy.org/
Apache License 2.0
866 stars 180 forks source link

backpressure, heartbeat datachannel, numGetters, misc. #2856

Closed jab closed 7 years ago

jab commented 7 years ago

2848 changed the numGetters tracking logic to check for ice connection state changes to the 'connected' and 'disconnected' states to update numGetters accordingly. This logic starts here: https://github.com/uProxy/uproxy/pull/2848/files#diff-1978b1ecb88355bcd34e7e58e5828653R198

But I just noticed #1511, in which @bemasc says: "In principle 'disconnected' may fire after 5 seconds of no response, but the connection may automatically transition back to the 'connected' state if pings start working again within 30 seconds." That would make the implementation above quite oversensitive.

I noticed our current code uses a heartbeat approach to track connection health (search peerconnection.ts for "heartbeat"). This PR swaps out the bootstrap datachannel for a heartbeat datachannel, and changes the numGetters logic to check whether we've gotten any heartbeats within a HEARTBEAT_TIMEOUT. This is preferable, no?

Some other small fixes are also included.

This change is Reviewable

jab commented 7 years ago

meant to say, I was in fact seeing numGetters drop to -1 with the previous logic, and no longer with this logic.

trevj commented 7 years ago

:+1:

jab commented 7 years ago

(Modified PR title to reflect that the backpressure implementation also landed in this PR.)

Upon merging trevj-socks-refactor into jab-zork-refactor and pushing, GitHub automatically marked this PR as merged since you'd already merged from jab-zork-refactor into trevj-socks-refactor. Will open a new PR for the SocksSession fixes.