From c9d2f4c5d24bcf5744b1053ab602c485cf38eaa3 Mon Sep 17 00:00:00 2001 From: HashimTheArab Date: Sat, 28 Feb 2026 18:07:24 -0500 Subject: [PATCH 1/3] Add StartContext to ICE and DTLS Transport Co-Authored-By: Claude Opus 4.6 --- dtlstransport.go | 17 ++++++++++++++++- dtlstransport_test.go | 2 +- icetransport.go | 17 +++++++++++++++-- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/dtlstransport.go b/dtlstransport.go index fe46daffbe4..c44ee913d7d 100644 --- a/dtlstransport.go +++ b/dtlstransport.go @@ -7,6 +7,7 @@ package webrtc import ( + "context" "crypto/ecdsa" "crypto/elliptic" "crypto/rand" @@ -302,6 +303,20 @@ func (t *DTLSTransport) role() DTLSRole { // Start DTLS transport negotiation with the parameters of the remote DTLS transport. func (t *DTLSTransport) Start(remoteParameters DTLSParameters) error { + ctx := context.Background() + if t.api.settingEngine.dtls.connectContextMaker != nil { + var cancel func() + ctx, cancel = t.api.settingEngine.dtls.connectContextMaker() + defer cancel() + } + + return t.StartContext(ctx, remoteParameters) +} + +// StartContext starts DTLS transport negotiation with the parameters of the remote DTLS +// transport. If the context is canceled before the DTLS handshake is complete, the handshake +// is interrupted and an error is returned. +func (t *DTLSTransport) StartContext(ctx context.Context, remoteParameters DTLSParameters) error { role, certificate, err := t.prepareStart(remoteParameters) if err != nil { return err @@ -320,7 +335,7 @@ func (t *DTLSTransport) Start(remoteParameters DTLSParameters) error { return t.failStart(err) } - if err = t.handshakeDTLS(dtlsConn); err != nil { + if err = dtlsConn.HandshakeContext(ctx); err != nil { dtlsEndpoint.SetOnClose(nil) _ = dtlsConn.Close() diff --git a/dtlstransport_test.go b/dtlstransport_test.go index efc98de73fa..fa8cb37e5ff 100644 --- a/dtlstransport_test.go +++ b/dtlstransport_test.go @@ -217,7 +217,7 @@ func (c *failingPacketConn) SetReadDeadline(time.Time) error { return nil } func (c *failingPacketConn) SetWriteDeadline(time.Time) error { return nil } func TestDTLSTransport_Start_ErrICEConnectionNotStarted(t *testing.T) { - transport := &DTLSTransport{state: DTLSTransportStateNew} + transport := &DTLSTransport{api: NewAPI(), state: DTLSTransportStateNew} err := transport.Start(DTLSParameters{Role: DTLSRoleServer}) assert.ErrorIs(t, err, errICEConnectionNotStarted) diff --git a/icetransport.go b/icetransport.go index df432d890bd..e2fca453f42 100644 --- a/icetransport.go +++ b/icetransport.go @@ -88,7 +88,20 @@ func NewICETransport(gatherer *ICEGatherer, loggerFactory logging.LoggerFactory) } // Start incoming connectivity checks based on its configured role. -func (t *ICETransport) Start(gatherer *ICEGatherer, params ICEParameters, role *ICERole) error { //nolint:cyclop +func (t *ICETransport) Start(gatherer *ICEGatherer, params ICEParameters, role *ICERole) error { + return t.StartContext(context.Background(), gatherer, params, role) +} + +// StartContext incoming connectivity checks based on its configured role. +// If the context is canceled, the ICE transport will stop. +// +//nolint:cyclop +func (t *ICETransport) StartContext( + ctx context.Context, + gatherer *ICEGatherer, + params ICEParameters, + role *ICERole, +) error { t.lock.Lock() defer t.lock.Unlock() @@ -135,7 +148,7 @@ func (t *ICETransport) Start(gatherer *ICEGatherer, params ICEParameters, role * } t.role = *role - ctx, ctxCancel := context.WithCancel(context.Background()) + ctx, ctxCancel := context.WithCancel(ctx) t.ctxCancel = ctxCancel // Drop the lock here to allow ICE candidates to be From 89ec395180903a35e39fe86ae03c65b8c66b9076 Mon Sep 17 00:00:00 2001 From: HashimTheArab Date: Wed, 3 Jun 2026 22:16:34 -0400 Subject: [PATCH 2/3] Close ICE agent when StartContext is canceled The review thread identified that pion/ice starts connectivity checks before context cancellation returns, leaving the agent running unless WebRTC closes it explicitly. This keeps the existing StartContext API shape and routes canceled starts through ICETransport Stop so the gatherer/agent are released. Constraint: PR review requested closing the ICE agent when the StartContext context is canceled after checks begin. Rejected: Changing pion/ice behavior in this PR | reviewer noted that belongs in the next major pion/ice version. Confidence: high Scope-risk: narrow Directive: Keep StartContext cancellation tied to ICETransport shutdown until pion/ice owns cancellation cleanup itself. Tested: go test ./ -run 'TestICETransport_StartContextClosesOnCancel|TestPeerConnection_Close$' -count=1; golangci-lint run --new-from-rev=HEAD; go test ./... --- icetransport.go | 6 ++++++ icetransport_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/icetransport.go b/icetransport.go index 49b115b1bc8..2d8e5ecc62a 100644 --- a/icetransport.go +++ b/icetransport.go @@ -174,6 +174,12 @@ func (t *ICETransport) StartContext( // Reacquire the lock to set the connection/mux t.lock.Lock() if err != nil { + if ctx.Err() != nil { + t.lock.Unlock() + _ = t.Stop() + t.lock.Lock() + } + return err } diff --git a/icetransport_test.go b/icetransport_test.go index 6540392a83d..172655603bf 100644 --- a/icetransport_test.go +++ b/icetransport_test.go @@ -6,6 +6,7 @@ package webrtc import ( + "context" "sync" "sync/atomic" "testing" @@ -15,6 +16,34 @@ import ( "github.com/stretchr/testify/assert" ) +func TestICETransport_StartContextClosesOnCancel(t *testing.T) { + lim := test.TimeOut(time.Second * 30) + defer lim.Stop() + + api := NewAPI() + gatherer, err := api.NewICEGatherer(ICEGatherOptions{}) + assert.NoError(t, err) + + remoteGatherer, err := api.NewICEGatherer(ICEGatherOptions{}) + assert.NoError(t, err) + + params, err := remoteGatherer.GetLocalParameters() + assert.NoError(t, err) + defer func() { + assert.NoError(t, remoteGatherer.Close()) + }() + + transport := api.NewICETransport(gatherer) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + controlling := ICERoleControlling + err = transport.StartContext(ctx, nil, params, &controlling) + assert.Error(t, err) + assert.Equal(t, ICEGathererStateClosed, gatherer.State()) + assert.Nil(t, gatherer.getAgent()) +} + func TestICETransport_OnConnectionStateChange(t *testing.T) { report := test.CheckRoutines(t) defer report() From deb41892f4deb8cc34e410794f4eac5794c3a4bb Mon Sep 17 00:00:00 2001 From: HashimTheArab Date: Thu, 4 Jun 2026 13:30:02 -0700 Subject: [PATCH 3/3] Return caller cancellation from ICE StartContext Callers that cancel StartContext should receive the context cancellation error they can inspect, while WebRTC still closes the ICE transport to release the agent. Non-context start failures now clear the temporary internal cancel function instead of leaving it attached to a start that failed. Constraint: PR review requested context cancellation semantics before merge. Rejected: Returning the pion/ice cancellation error on caller cancellation | it hides the caller's context.Canceled or DeadlineExceeded signal. Confidence: high Scope-risk: narrow Directive: Keep cancellation-return semantics aligned with context-aware Go APIs. Tested: go test ./ -run 'TestICETransport_StartContextClosesOnCancel|TestPeerConnection_Close$' -count=1; golangci-lint run --new-from-rev=HEAD Not-tested: go test ./... did not complete cleanly locally due existing network-sensitive mDNS/IPv6 failures in TestPeerConnectionTrickle and Test_IPv6. --- icetransport.go | 7 ++++++- icetransport_test.go | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/icetransport.go b/icetransport.go index 2d8e5ecc62a..bf8f874eb8b 100644 --- a/icetransport.go +++ b/icetransport.go @@ -174,12 +174,17 @@ func (t *ICETransport) StartContext( // Reacquire the lock to set the connection/mux t.lock.Lock() if err != nil { - if ctx.Err() != nil { + if ctxErr := ctx.Err(); ctxErr != nil { t.lock.Unlock() _ = t.Stop() t.lock.Lock() + + return ctxErr } + ctxCancel() + t.ctxCancel = nil + return err } diff --git a/icetransport_test.go b/icetransport_test.go index 172655603bf..1bf6f1da07b 100644 --- a/icetransport_test.go +++ b/icetransport_test.go @@ -39,7 +39,7 @@ func TestICETransport_StartContextClosesOnCancel(t *testing.T) { controlling := ICERoleControlling err = transport.StartContext(ctx, nil, params, &controlling) - assert.Error(t, err) + assert.ErrorIs(t, err, context.Canceled) assert.Equal(t, ICEGathererStateClosed, gatherer.State()) assert.Nil(t, gatherer.getAgent()) }