ICE: Dedupe servers and track the server index on STUN and TURN requests

[kwy404]

When a STUN binding or TURN Allocate request timed out, the native ICE client found the current server by address and moved to the next entry, so a list with a repeated address (for example several hostnames resolving to one IP) kept retrying the same entry forever. Duplicate addresses are now skipped when building the STUN and TURN server lists (keeping the first TURN entry's credentials), and each request carries the index of its server so discovery, keepalive and TURN failover always move forward. Adds failure tests that list the dead STUN server and the dead TURN server twice and check that each is tried only once.

Fixes #437

[tintinhamans]

TURN has the same bug: STUNRequestCallback_AllocateRelay finds the current server with index_of( m_vecTURNServers, addr ), so A, A loops forever. Dedupe fits TURN less well since credentials are per entry. Tracking the server index on the request would fix both.

[tintinhamans]

Thanks for the quick update, I just had a quick look and then noticed:

• Keepalive still uses index_of, so without the dedupe, A, A, B never reaches B.
• TURN credentials are also looked up by address (401 retry, refresh), so duplicate entries never get their own credentials anyway.

Maybe keep the dedupe for both as well as the index tracking.

[kwy404]

@tintinhamans Thanks, good point. Switched to tracking the server index on the request in a7693c1 (plus 6924dbc for the keepalive path), so the STUN and TURN retry paths move to the next entry even when addresses repeat, and the dedupe is gone.

Tests: added a TURN case next to the STUN one, listing the dead TURN server twice. Both use a longer connection timeout so a third attempt would show up: on upstream master they fail (12 retransmits and 4 allocate sends instead of 8 and 2), and with this change they pass. test_crypto, test_connection suite-quick, the other STUN/TURN failure cases and the TURN refresh relay test also pass on Windows (MSVC).

[kwy404]

@tintinhamans Thanks, good points. Keepalive was already switched to the tracked index in 6924dbc, and ff69e70 brings back the dedupe for both STUN and TURN, keeping the first entry's credentials for TURN since those are looked up by address. The index tracking stays so failover always moves forward. The duplicate entry tests now expect the dead server to be tried only once, and they still fail on master.

[tintinhamans]

LGTM, maybe squash

[kwy404]

Thanks for the review. Squashed into a single commit (1e819e3), the code is unchanged.