From 4e4b113e83eb495924d8673da33ed9783210f6ec Mon Sep 17 00:00:00 2001 From: Sean DuBois Date: Tue, 2 Jan 2024 11:43:22 -0500 Subject: [PATCH 1/9] Remove 'Generate Authors' workflow pion/.goassets#185 --- .github/workflows/generate-authors.yml | 23 ----------------- AUTHORS.txt | 35 -------------------------- 2 files changed, 58 deletions(-) delete mode 100644 .github/workflows/generate-authors.yml delete mode 100644 AUTHORS.txt diff --git a/.github/workflows/generate-authors.yml b/.github/workflows/generate-authors.yml deleted file mode 100644 index ec7446c8..00000000 --- a/.github/workflows/generate-authors.yml +++ /dev/null @@ -1,23 +0,0 @@ -# -# DO NOT EDIT THIS FILE -# -# It is automatically copied from https://github.com/pion/.goassets repository. -# If this repository should have package specific CI config, -# remove the repository name from .goassets/.github/workflows/assets-sync.yml. -# -# If you want to update the shared CI config, send a PR to -# https://github.com/pion/.goassets instead of this repository. -# -# SPDX-FileCopyrightText: 2023 The Pion community -# SPDX-License-Identifier: MIT - -name: Generate Authors - -on: - pull_request: - -jobs: - generate: - uses: pion/.goassets/.github/workflows/generate-authors.reusable.yml@master - secrets: - token: ${{ secrets.PIONBOT_PRIVATE_KEY }} diff --git a/AUTHORS.txt b/AUTHORS.txt deleted file mode 100644 index ae88cf02..00000000 --- a/AUTHORS.txt +++ /dev/null @@ -1,35 +0,0 @@ -# Thank you to everyone that made Pion possible. If you are interested in contributing -# we would love to have you https://github.com/pion/webrtc/wiki/Contributing -# -# This file is auto generated, using git to list all individuals contributors. -# see https://github.com/pion/.goassets/blob/master/scripts/generate-authors.sh for the scripting -Aaron France -Adrian Cable -Atsushi Watanabe -backkem -Cecylia Bocovich -chenkaiC4 -Eric Daniels -Hugo Arregui -Hugo Arregui -Jerko Steiner -Jerry Tao -John Bradley -Juliusz Chroboczek -Konstantin Itskov -Lukas Herman -Luke Curley -Michael MacDonald -ronan -Sam Lancia -Sean DuBois -Sean DuBois -Steffen -Steffen Vogel -Teddy -Will Forcey -Yutaka Takeda -ZHENK - -# List of contributors not appearing in Git history - From 58f271d581cb3c2fe6ec4f78a30d2f1c150708ed Mon Sep 17 00:00:00 2001 From: Sean DuBois Date: Tue, 2 Jan 2024 12:51:15 -0500 Subject: [PATCH 2/9] Remove 'AUTHORS.txt' from README.md Relates to pion/.goassets#185 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 3ca23445..a407c46a 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ We are always looking to support **your projects**. Please reach out if you have If you need commercial support or don't want to use public methods you can contact us at [team@pion.ly](mailto:team@pion.ly) ### Contributing -Check out the [contributing wiki](https://github.com/pion/webrtc/wiki/Contributing) to join the group of amazing people making this project possible: [AUTHORS.txt](./AUTHORS.txt) +Check out the [contributing wiki](https://github.com/pion/webrtc/wiki/Contributing) to join the group of amazing people making this project possible ### License MIT License - see [LICENSE](LICENSE) for full text From 0dd7930570890b98ed4f13449db763e995296bfc Mon Sep 17 00:00:00 2001 From: Pion <59523206+pionbot@users.noreply.github.com> Date: Tue, 2 Jan 2024 19:16:28 +0000 Subject: [PATCH 3/9] Update CI configs to v0.11.0 Update lint scripts and CI configs. --- .golangci.yml | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 4e3eddf4..6dd80c80 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -29,7 +29,6 @@ linters: - bodyclose # checks whether HTTP response body is closed successfully - contextcheck # check the function whether use a non-inherited context - decorder # check declaration order and count of types, constants, variables and functions - - depguard # Go linter that checks if package imports are in a list of acceptable packages - dogsled # Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) - dupl # Tool for code clone detection - durationcheck # check for two durations multiplied together @@ -63,7 +62,6 @@ linters: - importas # Enforces consistent import aliases - ineffassign # Detects when assignments to existing variables are not used - misspell # Finds commonly misspelled English words in comments - - nakedret # Finds naked returns in functions greater than a specified function length - nilerr # Finds the code that returns nil even if it checks that the error is not nil. - nilnil # Checks that there is no simultaneous return of `nil` error and an invalid value. - noctx # noctx finds sending http request without context.Context @@ -81,6 +79,7 @@ linters: - wastedassign # wastedassign finds wasted assignment statements - whitespace # Tool for detection of leading and trailing whitespace disable: + - depguard # Go linter that checks if package imports are in a list of acceptable packages - containedctx # containedctx is a linter that detects struct contained context.Context field - cyclop # checks function and package cyclomatic complexity - exhaustivestruct # Checks if all struct's fields are initialized @@ -94,6 +93,7 @@ linters: - maintidx # maintidx measures the maintainability index of each function. - makezero # Finds slice declarations with non-zero initial length - maligned # Tool to detect Go structs that would take less memory if their fields were sorted + - nakedret # Finds naked returns in functions greater than a specified function length - nestif # Reports deeply nested if statements - nlreturn # nlreturn checks for a new line before return and branch statements to increase code clarity - nolintlint # Reports ill-formed or insufficient nolint directives @@ -111,22 +111,11 @@ linters: issues: exclude-use-default: false exclude-rules: - # Allow complex tests, better to be self contained - - path: _test\.go + # Allow complex tests and examples, better to be self contained + - path: (examples|main\.go|_test\.go) linters: - - gocognit - forbidigo - - # Allow complex main function in examples - - path: examples - text: "of func `main` is high" - linters: - gocognit - - # Allow forbidden identifiers in examples - - path: examples - linters: - - forbidigo # Allow forbidden identifiers in CLI commands - path: cmd From 1abdf2d7b2881b90392ceb9176fe4b90f4a0b8d8 Mon Sep 17 00:00:00 2001 From: Pion <59523206+pionbot@users.noreply.github.com> Date: Fri, 5 Jan 2024 00:05:24 +0000 Subject: [PATCH 4/9] Update CI configs to v0.11.3 Update lint scripts and CI configs. --- .github/workflows/test.yaml | 4 ++-- .github/workflows/tidy-check.yaml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 31aada4a..c8294ef1 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -23,7 +23,7 @@ jobs: uses: pion/.goassets/.github/workflows/test.reusable.yml@master strategy: matrix: - go: ['1.20', '1.19'] # auto-update/supported-go-version-list + go: ['1.21', '1.20'] # auto-update/supported-go-version-list fail-fast: false with: go-version: ${{ matrix.go }} @@ -32,7 +32,7 @@ jobs: uses: pion/.goassets/.github/workflows/test-i386.reusable.yml@master strategy: matrix: - go: ['1.20', '1.19'] # auto-update/supported-go-version-list + go: ['1.21', '1.20'] # auto-update/supported-go-version-list fail-fast: false with: go-version: ${{ matrix.go }} diff --git a/.github/workflows/tidy-check.yaml b/.github/workflows/tidy-check.yaml index 4d346d4f..33d6b507 100644 --- a/.github/workflows/tidy-check.yaml +++ b/.github/workflows/tidy-check.yaml @@ -22,4 +22,4 @@ jobs: tidy: uses: pion/.goassets/.github/workflows/tidy-check.reusable.yml@master with: - go-version: '1.20' # auto-update/latest-go-version + go-version: '1.21' # auto-update/latest-go-version From daee3aa23c268924ce09396907df360aa3cae2b1 Mon Sep 17 00:00:00 2001 From: Yutaka Takeda Date: Thu, 28 Dec 2023 18:04:31 -0800 Subject: [PATCH 5/9] Fix goroutine leakage Added reproducible leakage check along with a fix. --- association_test.go | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/association_test.go b/association_test.go index 12875b94..e50f2f87 100644 --- a/association_test.go +++ b/association_test.go @@ -2093,8 +2093,28 @@ func TestAssocDelayedAck(t *testing.T) { }) } +func checkGoroutineLeaks(t *testing.T) { + // Get the count of goroutines at the start of the test. + initialGoroutines := runtime.NumGoroutine() + // Register a cleanup function to run after the test completes. + t.Cleanup(func() { + // Allow for up to 1 second for all goroutines to finish. + for i := 0; i < 10; i++ { + time.Sleep(100 * time.Millisecond) + if goroutines := runtime.NumGoroutine(); goroutines <= initialGoroutines { + return + } + } + + // If we've gotten this far, not all goroutines have finished. + t.Errorf("leaked %d goroutines", runtime.NumGoroutine()-initialGoroutines) + }) +} + func TestAssocReset(t *testing.T) { t.Run("Close one way", func(t *testing.T) { + checkGoroutineLeaks(t) + lim := test.TimeOut(time.Second * 10) defer lim.Stop() @@ -2156,6 +2176,8 @@ func TestAssocReset(t *testing.T) { }) t.Run("Close both ways", func(t *testing.T) { + checkGoroutineLeaks(t) + lim := test.TimeOut(time.Second * 10) defer lim.Stop() @@ -2173,6 +2195,7 @@ func TestAssocReset(t *testing.T) { assert.Equal(t, 0, a0.bufferedAmount(), "incorrect bufferedAmount") + // send a message from s0 to s1 n, err := s0.WriteSCTP([]byte(msg), PayloadTypeWebRTCBinary) if err != nil { assert.FailNow(t, "failed due to earlier error") @@ -2180,7 +2203,8 @@ func TestAssocReset(t *testing.T) { assert.Equal(t, len(msg), n, "unexpected length of received data") assert.Equal(t, len(msg), a0.bufferedAmount(), "incorrect bufferedAmount") - err = s0.Close() // send reset + // close s0 as soon as the message is sent + err = s0.Close() if err != nil { t.Error(err) } @@ -2213,7 +2237,8 @@ func TestAssocReset(t *testing.T) { } } - err = s1.Close() // send reset + // send reset from s1 + err = s1.Close() if err != nil { t.Error(err) } @@ -2222,7 +2247,10 @@ func TestAssocReset(t *testing.T) { for { _, _, err = s0.ReadSCTP(buf) assert.Equal(t, io.EOF, err, "should be EOF") - doneCh <- err + if err != nil { + doneCh <- err + return + } } }() From d69aa98ca8f9f9f789c03cd7080ec5e0f850c6ca Mon Sep 17 00:00:00 2001 From: Yutaka Date: Fri, 29 Dec 2023 02:35:51 +0000 Subject: [PATCH 6/9] Improved UDPConn pair and log messages Relates to #270 --- association.go | 7 +++ association_test.go | 114 ++++++++++++++++++++++++++++++-------------- 2 files changed, 86 insertions(+), 35 deletions(-) diff --git a/association.go b/association.go index ce3725ec..f0f1d7be 100644 --- a/association.go +++ b/association.go @@ -254,10 +254,17 @@ func Server(config Config) (*Association, error) { // Client opens a SCTP stream over a conn func Client(config Config) (*Association, error) { + return createClientWithContext(context.Background(), config) +} + +func createClientWithContext(ctx context.Context, config Config) (*Association, error) { a := createAssociation(config) a.init(true) select { + case <-ctx.Done(): + a.log.Errorf("[%s] client handshake canceled: state=%s", a.name, getAssociationStateString(a.getState())) + return nil, ctx.Err() case err := <-a.handshakeCompletedCh: if err != nil { return nil, err diff --git a/association_test.go b/association_test.go index e50f2f87..7bfcce74 100644 --- a/association_test.go +++ b/association_test.go @@ -2570,65 +2570,106 @@ func TestAssocMaxMessageSize(t *testing.T) { }) } -func createAssocs(t *testing.T) (a1, a2 *Association) { - addr1 := &net.UDPAddr{ - IP: net.IP{127, 0, 0, 1}, - Port: 1234, +// crateUDPConnPair creates a pair of net.UDPConn objects that are connected with each other +func createUDPConnPair(t *testing.T) (*net.UDPConn, *net.UDPConn, error) { + udp1, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.ParseIP("127.0.0.1")}) + if err != nil { + return nil, nil, err } + addr1, ok := udp1.LocalAddr().(*net.UDPAddr) + require.True(t, ok) + err = udp1.Close() + require.NoError(t, err) - addr2 := &net.UDPAddr{ - IP: net.IP{127, 0, 0, 1}, - Port: 5678, + udp2, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.ParseIP("127.0.0.1")}) + if err != nil { + return nil, nil, err } + addr2, ok := udp2.LocalAddr().(*net.UDPAddr) + require.True(t, ok) + err = udp2.Close() + require.NoError(t, err) - udp1, err := net.DialUDP("udp", addr1, addr2) + udp1, err = net.DialUDP("udp", addr1, addr2) if err != nil { - panic(err) + return nil, nil, err } - udp2, err := net.DialUDP("udp", addr2, addr1) + udp2, err = net.DialUDP("udp", addr2, addr1) if err != nil { - panic(err) + return nil, nil, err + } + + return udp1, udp2, nil +} + +func createAssocs(t *testing.T) (*Association, *Association, error) { + udp1, udp2, err := createUDPConnPair(t) + if err != nil { + return nil, nil, err } loggerFactory := logging.NewDefaultLoggerFactory() - a1Chan := make(chan *Association) - a2Chan := make(chan *Association) + a1Chan := make(chan interface{}) + a2Chan := make(chan interface{}) + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() go func() { - a, err := Client(Config{ + a, err2 := createClientWithContext(ctx, Config{ NetConn: udp1, LoggerFactory: loggerFactory, }) - require.NoError(t, err) - - a1Chan <- a + if err2 != nil { + a1Chan <- err2 + } else { + a1Chan <- a + } }() go func() { - a, err := Client(Config{ + a, err2 := createClientWithContext(ctx, Config{ NetConn: udp2, LoggerFactory: loggerFactory, }) - require.NoError(t, err) - - a2Chan <- a + if err2 != nil { + a2Chan <- err2 + } else { + a2Chan <- a + } }() - select { - case a1 = <-a1Chan: - case <-time.After(time.Second): - assert.Fail(t, "timed out waiting for a1") - } + var a1 *Association + var a2 *Association - select { - case a2 = <-a2Chan: - case <-time.After(time.Second): - assert.Fail(t, "timed out waiting for a2") +loop: + for { + select { + case v1 := <-a1Chan: + switch v := v1.(type) { + case *Association: + a1 = v + if a2 != nil { + break loop + } + case error: + return nil, nil, v + } + case v2 := <-a2Chan: + switch v := v2.(type) { + case *Association: + a2 = v + if a1 != nil { + break loop + } + case error: + return nil, nil, v + } + } } - - return a1, a2 + return a1, a2, nil } func TestAssociation_Shutdown(t *testing.T) { @@ -2640,7 +2681,8 @@ func TestAssociation_Shutdown(t *testing.T) { assert.Equal(t, n0, runtime.NumGoroutine(), "goroutine is leaked") }() - a1, a2 := createAssocs(t) + a1, a2, err := createAssocs(t) + require.NoError(t, err) s11, err := a1.OpenStream(1, PayloadTypeWebRTCString) require.NoError(t, err) @@ -2683,7 +2725,8 @@ func TestAssociation_ShutdownDuringWrite(t *testing.T) { assert.Equal(t, n0, runtime.NumGoroutine(), "goroutine is leaked") }() - a1, a2 := createAssocs(t) + a1, a2, err := createAssocs(t) + require.NoError(t, err) s11, err := a1.OpenStream(1, PayloadTypeWebRTCString) require.NoError(t, err) @@ -2899,7 +2942,8 @@ func TestAssociation_Abort(t *testing.T) { assert.Equal(t, n0, runtime.NumGoroutine(), "goroutine is leaked") }() - a1, a2 := createAssocs(t) + a1, a2, err := createAssocs(t) + require.NoError(t, err) s11, err := a1.OpenStream(1, PayloadTypeWebRTCString) require.NoError(t, err) From 60e10c95530eab6cb4435dd694ce764722f2fd09 Mon Sep 17 00:00:00 2001 From: Yutaka Takeda Date: Sat, 30 Dec 2023 12:29:27 -0800 Subject: [PATCH 7/9] Fix false-positive goroutine leak error Use checkGoroutineLeaks everywhere. --- association_test.go | 34 +++++----------------------------- 1 file changed, 5 insertions(+), 29 deletions(-) diff --git a/association_test.go b/association_test.go index 7bfcce74..25f2c895 100644 --- a/association_test.go +++ b/association_test.go @@ -2382,8 +2382,7 @@ func (c *fakeEchoConn) SetWriteDeadline(time.Time) error { return nil } func TestRoutineLeak(t *testing.T) { loggerFactory := logging.NewDefaultLoggerFactory() t.Run("Close failed", func(t *testing.T) { - runtime.GC() - n0 := runtime.NumGoroutine() + checkGoroutineLeaks(t) conn := newFakeEchoConn(io.EOF) a, err := Client(Config{NetConn: conn, LoggerFactory: loggerFactory}) @@ -2403,12 +2402,9 @@ func TestRoutineLeak(t *testing.T) { t.Errorf("closeWriteLoopCh is expected to be closed, but not") } _ = a - runtime.GC() - assert.Equal(t, n0, runtime.NumGoroutine(), "goroutine is leaked") }) t.Run("Connection closed by remote host", func(t *testing.T) { - runtime.GC() - n0 := runtime.NumGoroutine() + checkGoroutineLeaks(t) conn := newFakeEchoConn(nil) a, err := Client(Config{NetConn: conn, LoggerFactory: loggerFactory}) @@ -2429,8 +2425,6 @@ func TestRoutineLeak(t *testing.T) { default: t.Errorf("closeWriteLoopCh is expected to be closed, but not") } - runtime.GC() - assert.Equal(t, n0, runtime.NumGoroutine(), "goroutine is leaked") }) } @@ -2673,13 +2667,7 @@ loop: } func TestAssociation_Shutdown(t *testing.T) { - runtime.GC() - n0 := runtime.NumGoroutine() - - defer func() { - runtime.GC() - assert.Equal(t, n0, runtime.NumGoroutine(), "goroutine is leaked") - }() + checkGoroutineLeaks(t) a1, a2, err := createAssocs(t) require.NoError(t, err) @@ -2717,13 +2705,7 @@ func TestAssociation_Shutdown(t *testing.T) { } func TestAssociation_ShutdownDuringWrite(t *testing.T) { - runtime.GC() - n0 := runtime.NumGoroutine() - - defer func() { - runtime.GC() - assert.Equal(t, n0, runtime.NumGoroutine(), "goroutine is leaked") - }() + checkGoroutineLeaks(t) a1, a2, err := createAssocs(t) require.NoError(t, err) @@ -2934,13 +2916,7 @@ func TestAssociation_HandlePacketInCookieWaitState(t *testing.T) { } func TestAssociation_Abort(t *testing.T) { - runtime.GC() - n0 := runtime.NumGoroutine() - - defer func() { - runtime.GC() - assert.Equal(t, n0, runtime.NumGoroutine(), "goroutine is leaked") - }() + checkGoroutineLeaks(t) a1, a2, err := createAssocs(t) require.NoError(t, err) From 8c8b66b4e506f3078f88b6ab0e396ea74277a9eb Mon Sep 17 00:00:00 2001 From: Yutaka Takeda Date: Sun, 31 Dec 2023 00:26:39 -0800 Subject: [PATCH 8/9] Use UDPConn wrapper instead of connected UDP Fixes #270 --- association.go | 1 + association_test.go | 121 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 114 insertions(+), 8 deletions(-) diff --git a/association.go b/association.go index f0f1d7be..507e3aad 100644 --- a/association.go +++ b/association.go @@ -264,6 +264,7 @@ func createClientWithContext(ctx context.Context, config Config) (*Association, select { case <-ctx.Done(): a.log.Errorf("[%s] client handshake canceled: state=%s", a.name, getAssociationStateString(a.getState())) + a.Close() // nolint:errcheck,gosec return nil, ctx.Err() case err := <-a.handshakeCompletedCh: if err != nil { diff --git a/association_test.go b/association_test.go index 25f2c895..82e590fb 100644 --- a/association_test.go +++ b/association_test.go @@ -2564,16 +2564,62 @@ func TestAssocMaxMessageSize(t *testing.T) { }) } +// udpConnWrapper wraps a *net.UDPConn and implements net.Conn interface. +type udpConnWrapper struct { + conn *net.UDPConn + remoteAddr net.Addr +} + +func newUDPConnWrapper(conn *net.UDPConn, remoteAddr net.Addr) net.Conn { + return &udpConnWrapper{ + conn: conn, + remoteAddr: remoteAddr, + } +} + +// Implement the net.Conn interface methods +func (w *udpConnWrapper) Read(b []byte) (n int, err error) { + // w.conn.ReadFrom(b) + n, _, err = w.conn.ReadFrom(b) + return n, err +} + +func (w *udpConnWrapper) Write(b []byte) (n int, err error) { + return w.conn.WriteTo(b, w.remoteAddr) +} + +func (w *udpConnWrapper) Close() error { + return w.conn.Close() +} + +func (w *udpConnWrapper) LocalAddr() net.Addr { + return w.conn.LocalAddr() +} + +func (w *udpConnWrapper) RemoteAddr() net.Addr { + return w.remoteAddr +} + +func (w *udpConnWrapper) SetDeadline(t time.Time) error { + return w.conn.SetDeadline(t) +} + +func (w *udpConnWrapper) SetReadDeadline(t time.Time) error { + return w.conn.SetReadDeadline(t) +} + +func (w *udpConnWrapper) SetWriteDeadline(t time.Time) error { + return w.conn.SetWriteDeadline(t) +} + // crateUDPConnPair creates a pair of net.UDPConn objects that are connected with each other -func createUDPConnPair(t *testing.T) (*net.UDPConn, *net.UDPConn, error) { +func createUDPConnPair(t *testing.T) (net.Conn, net.Conn, error) { udp1, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.ParseIP("127.0.0.1")}) if err != nil { return nil, nil, err } addr1, ok := udp1.LocalAddr().(*net.UDPAddr) require.True(t, ok) - err = udp1.Close() - require.NoError(t, err) udp2, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.ParseIP("127.0.0.1")}) if err != nil { @@ -2581,20 +2627,18 @@ func createUDPConnPair(t *testing.T) (*net.UDPConn, *net.UDPConn, error) { } addr2, ok := udp2.LocalAddr().(*net.UDPAddr) require.True(t, ok) - err = udp2.Close() - require.NoError(t, err) - udp1, err = net.DialUDP("udp", addr1, addr2) + conn1 := newUDPConnWrapper(udp1, addr2) if err != nil { return nil, nil, err } - udp2, err = net.DialUDP("udp", addr2, addr1) + conn2 := newUDPConnWrapper(udp2, addr1) if err != nil { return nil, nil, err } - return udp1, udp2, nil + return conn1, conn2, nil } func createAssocs(t *testing.T) (*Association, *Association, error) { @@ -2952,3 +2996,64 @@ func TestAssociation_Abort(t *testing.T) { assert.Equal(t, i, 0, "expected no data read") assert.Error(t, err, "User Initiated Abort: 1234", "expected abort reason") } + +// TestAssociation_createClientWithContext tests that the client is closed when the context is canceled. +func TestAssociation_createClientWithContext(t *testing.T) { + checkGoroutineLeaks(t) + + udp1, udp2, err := createUDPConnPair(t) + require.NoError(t, err) + + loggerFactory := logging.NewDefaultLoggerFactory() + + errCh1 := make(chan error) + errCh2 := make(chan error) + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + + go func() { + _, err2 := createClientWithContext(ctx, Config{ + NetConn: udp1, + LoggerFactory: loggerFactory, + }) + if err2 != nil { + errCh1 <- err2 + } else { + errCh1 <- nil + } + }() + + go func() { + _, err2 := createClientWithContext(ctx, Config{ + NetConn: udp2, + LoggerFactory: loggerFactory, + }) + if err2 != nil { + errCh2 <- err2 + } else { + errCh2 <- nil + } + }() + + // Cancel the context immediately + cancel() + + var err1 error + var err2 error +loop: + for { + select { + case err1 = <-errCh1: + if err1 != nil && err2 != nil { + break loop + } + case err2 = <-errCh2: + if err1 != nil && err2 != nil { + break loop + } + } + } + + assert.Error(t, err1, "context canceled") + assert.Error(t, err2, "context canceled") +} From 4bb25dc25ce1935b88e820ddd95ddde621c60d75 Mon Sep 17 00:00:00 2001 From: Yutaka Takeda Date: Sun, 31 Dec 2023 20:46:20 -0800 Subject: [PATCH 9/9] Use dumbConn2 in createAssocs This is avoid the issue with unexpected errors occur on Ubuntu linux. Relates to #270 --- association_test.go | 139 ++++++++++++++++++++++++-------------------- 1 file changed, 77 insertions(+), 62 deletions(-) diff --git a/association_test.go b/association_test.go index 82e590fb..7e884772 100644 --- a/association_test.go +++ b/association_test.go @@ -2564,88 +2564,104 @@ func TestAssocMaxMessageSize(t *testing.T) { }) } -// udpConnWrapper wraps a *net.UDPConn and implements net.Conn interface. -type udpConnWrapper struct { - conn *net.UDPConn - remoteAddr net.Addr +type dumbConnInboundHandler func([]byte) + +type dumbConn2 struct { + net.Conn + packets [][]byte + closed bool + localAddr net.Addr + remoteAddr net.Addr + remoteInboundHandler dumbConnInboundHandler + mutex sync.Mutex + cond *sync.Cond } -func newUDPConnWrapper(conn *net.UDPConn, remoteAddr net.Addr) net.Conn { - return &udpConnWrapper{ - conn: conn, +func newDumbConn2(localAddr, remoteAddr net.Addr) *dumbConn2 { + c := &dumbConn2{ + packets: [][]byte{}, + localAddr: localAddr, remoteAddr: remoteAddr, } + c.cond = sync.NewCond(&c.mutex) + return c } // Implement the net.Conn interface methods -func (w *udpConnWrapper) Read(b []byte) (n int, err error) { - // w.conn.ReadFrom(b) - n, _, err = w.conn.ReadFrom(b) - return n, err -} +func (c *dumbConn2) Read(b []byte) (n int, err error) { + c.mutex.Lock() + defer c.mutex.Unlock() -func (w *udpConnWrapper) Write(b []byte) (n int, err error) { - return w.conn.WriteTo(b, w.remoteAddr) -} + for { + if len(c.packets) > 0 { + packet := c.packets[0] + c.packets = c.packets[1:] + n := copy(b, packet) + return n, nil + } -func (w *udpConnWrapper) Close() error { - return w.conn.Close() -} + if c.closed { + return 0, io.EOF + } -func (w *udpConnWrapper) LocalAddr() net.Addr { - return w.conn.LocalAddr() + c.cond.Wait() + } } -func (w *udpConnWrapper) RemoteAddr() net.Addr { - return w.remoteAddr -} +func (c *dumbConn2) Write(b []byte) (int, error) { + c.mutex.Lock() + closed := c.closed + c.mutex.Unlock() -func (w *udpConnWrapper) SetDeadline(t time.Time) error { - return w.conn.SetDeadline(t) + if closed { + return 0, &net.OpError{Op: "write", Net: "udp", Addr: c.remoteAddr, Err: net.ErrClosed} + } + c.remoteInboundHandler(b) + return len(b), nil } -func (w *udpConnWrapper) SetReadDeadline(t time.Time) error { - return w.conn.SetReadDeadline(t) -} +func (c *dumbConn2) Close() error { + c.mutex.Lock() + defer c.mutex.Unlock() -func (w *udpConnWrapper) SetWriteDeadline(t time.Time) error { - return w.conn.SetWriteDeadline(t) + c.closed = true + c.cond.Signal() + return nil } -// crateUDPConnPair creates a pair of net.UDPConn objects that are connected with each other -func createUDPConnPair(t *testing.T) (net.Conn, net.Conn, error) { - udp1, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.ParseIP("127.0.0.1")}) - if err != nil { - return nil, nil, err - } - addr1, ok := udp1.LocalAddr().(*net.UDPAddr) - require.True(t, ok) +func (c *dumbConn2) LocalAddr() net.Addr { + // Unused by Association + return c.localAddr +} - udp2, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.ParseIP("127.0.0.1")}) - if err != nil { - return nil, nil, err - } - addr2, ok := udp2.LocalAddr().(*net.UDPAddr) - require.True(t, ok) +func (c *dumbConn2) RemoteAddr() net.Addr { + // Unused by Association + return c.remoteAddr +} - conn1 := newUDPConnWrapper(udp1, addr2) - if err != nil { - return nil, nil, err - } +func (c *dumbConn2) inboundHandler(packet []byte) { + c.mutex.Lock() + defer c.mutex.Unlock() - conn2 := newUDPConnWrapper(udp2, addr1) - if err != nil { - return nil, nil, err + if !c.closed { + c.packets = append(c.packets, packet) + c.cond.Signal() } +} - return conn1, conn2, nil +// crateUDPConnPair creates a pair of net.UDPConn objects that are connected with each other +func createUDPConnPair() (net.Conn, net.Conn) { + addr1 := &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 1234} + addr2 := &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 5678} + conn1 := newDumbConn2(addr1, addr2) + conn2 := newDumbConn2(addr2, addr1) + conn1.remoteInboundHandler = conn2.inboundHandler + conn2.remoteInboundHandler = conn1.inboundHandler + return conn1, conn2 } -func createAssocs(t *testing.T) (*Association, *Association, error) { - udp1, udp2, err := createUDPConnPair(t) - if err != nil { - return nil, nil, err - } +func createAssocs() (*Association, *Association, error) { + udp1, udp2 := createUDPConnPair() loggerFactory := logging.NewDefaultLoggerFactory() @@ -2713,7 +2729,7 @@ loop: func TestAssociation_Shutdown(t *testing.T) { checkGoroutineLeaks(t) - a1, a2, err := createAssocs(t) + a1, a2, err := createAssocs() require.NoError(t, err) s11, err := a1.OpenStream(1, PayloadTypeWebRTCString) @@ -2751,7 +2767,7 @@ func TestAssociation_Shutdown(t *testing.T) { func TestAssociation_ShutdownDuringWrite(t *testing.T) { checkGoroutineLeaks(t) - a1, a2, err := createAssocs(t) + a1, a2, err := createAssocs() require.NoError(t, err) s11, err := a1.OpenStream(1, PayloadTypeWebRTCString) @@ -2962,7 +2978,7 @@ func TestAssociation_HandlePacketInCookieWaitState(t *testing.T) { func TestAssociation_Abort(t *testing.T) { checkGoroutineLeaks(t) - a1, a2, err := createAssocs(t) + a1, a2, err := createAssocs() require.NoError(t, err) s11, err := a1.OpenStream(1, PayloadTypeWebRTCString) @@ -3001,8 +3017,7 @@ func TestAssociation_Abort(t *testing.T) { func TestAssociation_createClientWithContext(t *testing.T) { checkGoroutineLeaks(t) - udp1, udp2, err := createUDPConnPair(t) - require.NoError(t, err) + udp1, udp2 := createUDPConnPair() loggerFactory := logging.NewDefaultLoggerFactory()