Skip to content

Upgrade pion/webrtc to v4#3098

Merged
sukunrt merged 3 commits intolibp2p:masterfrom
badgooooor:chore/upgrade-pion-webrtc
Dec 25, 2024
Merged

Upgrade pion/webrtc to v4#3098
sukunrt merged 3 commits intolibp2p:masterfrom
badgooooor:chore/upgrade-pion-webrtc

Conversation

@badgooooor
Copy link
Copy Markdown
Contributor

@badgooooor badgooooor commented Dec 10, 2024

Description

p-shahi
p-shahi previously requested changes Dec 16, 2024
View reviewed changes
Comment thread go.mod Outdated
@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
Closed
@badgooooor badgooooor requested a review from p-shahi December 16, 2024 15:23
@p-shahi
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

badgooooor commented Dec 17, 2024

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
Copy Markdown
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
@sukunrt sukunrt force-pushed the chore/upgrade-pion-webrtc branch from e14b7fa to 78e750c Compare December 25, 2024 11:57
@sukunrt sukunrt force-pushed the chore/upgrade-pion-webrtc branch from 78e750c to e4c0a07 Compare December 25, 2024 12:19
sukunrt
sukunrt approved these changes Dec 25, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@sukunrt sukunrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @badgooooor ❤️

@sukunrt sukunrt merged commit 8f6d98d into libp2p:master Dec 25, 2024
2color added a commit that referenced this pull request Jan 24, 2025
@2color
Merge remote-tracking branch 'origin/marco/bring-go-nat-home' into de…
1bd76eb 
…bug-nat-port-mapping

* origin/marco/bring-go-nat-home: (75 commits)
  go mod tidy
  reference internal package for mockgen
  mod tidy test-plans
  Make changes to internal nat library
  ci: move to actions/upload-artifact@v4 (#3152)
  tcpreuse: fix rcmgr accounting when tcp metrics are enabled (#3142)
  fix(net/nat): data race problem of `extAddr` (#3140)
  test: fix failing test (#3141)
  quicreuse: make it possible to use an application-constructed quic.Transport (#3122)
  nat: ignore mapping if external port is 0 (#3094)
  tcpreuse: error on using tcpreuse with pnet (#3129)
  chore: Update contribution guidelines (#3134)
  tcp: fix metrics test build directive (#3052)
  webrtc: upgrade pion/webrtc to v4 (#3098)
  ci: get back on the main release track of release checker (#3117)
  webtransport: fix docstring comment for getCurrentBucketStartTime
  chore: release v0.38.1 (#3114)
  fix(httpauth): Correctly handle concurrent requests on server (#3111)
  ci: Install specific protoc version when generating protobufs (#3112)
  fix(autorelay): Move relayFinder peer disconnect cleanup to separate goroutine (#3105)
  ...
@BrewTestBot BrewTestBot mentioned this pull request Mar 20, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sukunrt sukunrt sukunrt approved these changes

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

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