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.

Sign up for GitHub

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

Jump to bottom

Upgrade pion/webrtc to v4 #3098

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Upgrade pion/webrtc to v4 #3098

wants to merge 3 commits into from
+92 −64

Conversation

badgooooor
Copy link

Description

p-shahi
p-shahi requested changes Dec 16, 2024
View reviewed changes
go.mod Outdated Show resolved Hide resolved
@p-shahi p-shahi linked an issue Dec 16, 2024 that may be closed by this pull request
webrtc: upgrade pion/webrtc to v4 #3072
Open
@badgooooor badgooooor requested a review from p-shahi December 16, 2024 15:23
@p-shahi
Copy link
Member

p-shahi commented Dec 16, 2024 •
edited
Loading

This webrtc v4 upgrade seems to make TestManyConnections racy
(There's no issue when I run 100 iterations of this test on master)

I was trying to narrow down what the problem was so I ran 100 iterations (go test ./p2p/transport/webrtc -run TestManyConnections -count=1) with the following values

const numListeners = 2
const dialersPerListener = 2
const connsPerDialer = 1

without any failure.

After increasing numListeners and dialersPerListener to 3 but keeping connections per dialer to 1:

const numListeners = 3
const dialersPerListener = 3
const connsPerDialer = 1

I can repro the issue seen in CI

❯ for i in {1..100}; do
  echo "Running iteration $i..."
  go test ./p2p/transport/webrtc -run TestManyConnections -count=1
done
Running iteration 1...
--- FAIL: TestManyConnections (0.03s)
    transport_test.go:982: dial failed: failed to secure outbound connection: error reading handshake message: proto: cannot parse invalid wire-format data
    transport_test.go:1006: failed: failed to secure outbound connection: error reading handshake message: proto: cannot parse invalid wire-format data
    transport_test.go:968: listener failed to accept conneciton: listener closed
    transport_test.go:968: listener failed to accept conneciton: listener closed
FAIL
FAIL	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.371s
FAIL
Running iteration 2...
ok  	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.585s
Running iteration 3...
ok  	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.370s
Running iteration 4...
ok  	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.583s
Running iteration 5...
--- FAIL: TestManyConnections (0.03s)
    transport_test.go:982: dial failed: failed to secure outbound connection: error reading handshake message: proto: cannot parse invalid wire-format data
    transport_test.go:1006: failed: failed to secure outbound connection: error reading handshake message: proto: cannot parse invalid wire-format data
    transport_test.go:968: listener failed to accept conneciton: listener closed
    transport_test.go:968: listener failed to accept conneciton: listener closed
FAIL
FAIL	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.369s
FAIL
Running iteration 6...
ok  	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.372s
Running iteration 7...
ok  	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.372s
Running iteration 8...
ok  	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.382s
Running iteration 9...
ok  	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.575s
Running iteration 10...
ok  	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.594s

cc @sukunrt @MarcoPolo

@badgooooor
Copy link
Author

I tried downsize the test parameters to something like this. It is still a bit flaky.

const numListeners = 3
const dialersPerListener = 3
const connsPerDialer = 6

Log:

Running tool: /usr/local/go/bin/go test -timeout 30s -run ^TestManyConnections$ github.com/libp2p/go-libp2p/p2p/transport/webrtc

=== RUN   TestManyConnections
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  0
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  1
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  2
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  3
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  4
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  5
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  6
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  7
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  8
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  9
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  10
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  11
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  12
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  13
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  14
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  15
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  16
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  17
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  18
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  19
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  20
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  21
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  22
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  23
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  24
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  25
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  26
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  27
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  28
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  29
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  30
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  31
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  32
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  33
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  34
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  35
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  36
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  37
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  38
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  39
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  40
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  41
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  42
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  43
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  44
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  45
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  46
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  47
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  48
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  49
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  50
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  51
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  52
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  53
--- PASS: TestManyConnections (0.24s)
PASS
ok      github.com/libp2p/go-libp2p/p2p/transport/webrtc        (cached)

cc. @p-shahi @sukunrt

@sukunrt
Copy link
Member

sukunrt commented Dec 23, 2024

Thanks @badgooooor. Looks like an issue in pion.

blocked on: pion/webrtc#2976

@sukunrt sukunrt self-assigned this Dec 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@p-shahi p-shahi Awaiting requested review from p-shahi

Requested changes must be addressed to merge this pull request.

Assignees

@sukunrt sukunrt

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

webrtc: upgrade pion/webrtc to v4
3 participants
@badgooooor @p-shahi @sukunrt