Skip to content

Commit

Permalink
Remove usage of ICETransport unexported fields
Browse files Browse the repository at this point in the history
There are some direct usages of unexported fields of ICETransport
from non ice-related methods. This would be problematic when ice
once ice related code is moved to a separate packet. Added proxy
methods to ICETransport to avoid this.
  • Loading branch information
masterada committed May 9, 2019
1 parent 6a0b702 commit 736e0bb
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 22 deletions.
10 changes: 4 additions & 6 deletions dtlstransport.go
Expand Up @@ -200,10 +200,9 @@ func (t *DTLSTransport) Start(remoteParameters DTLSParameters) error {
return err
}

mx := t.iceTransport.mux
dtlsEndpoint := mx.NewEndpoint(mux.MatchDTLS)
t.srtpEndpoint = mx.NewEndpoint(mux.MatchSRTP)
t.srtcpEndpoint = mx.NewEndpoint(mux.MatchSRTCP)
dtlsEndpoint := t.iceTransport.NewEndpoint(mux.MatchDTLS)
t.srtpEndpoint = t.iceTransport.NewEndpoint(mux.MatchSRTP)
t.srtcpEndpoint = t.iceTransport.NewEndpoint(mux.MatchSRTCP)

// TODO: handle multiple certs
cert := t.certificates[0]
Expand Down Expand Up @@ -291,8 +290,7 @@ func (t *DTLSTransport) validateFingerPrint(remoteParameters DTLSParameters, rem

func (t *DTLSTransport) ensureICEConn() error {
if t.iceTransport == nil ||
t.iceTransport.conn == nil ||
t.iceTransport.mux == nil {
t.iceTransport.State() == ICETransportStateNew {
return errors.New("ICE connection not started")
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Expand Up @@ -5,7 +5,7 @@ go 1.12
require (
github.com/pion/datachannel v1.4.3
github.com/pion/dtls v1.3.4
github.com/pion/ice v0.2.6
github.com/pion/ice v0.2.7
github.com/pion/logging v0.2.1
github.com/pion/quic v0.1.1
github.com/pion/rtcp v1.2.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Expand Up @@ -24,8 +24,8 @@ github.com/pion/datachannel v1.4.3 h1:tqS6YiqqAiFCxGGhvn1K7fHEzemK9Aov025dE/isGF
github.com/pion/datachannel v1.4.3/go.mod h1:SpMJbuu8v+qbA94m6lWQwSdCf8JKQvgmdSHDNtcbe+w=
github.com/pion/dtls v1.3.4 h1:MdOMsCfd44m2iTrxtkzA6UndvYVjLWWjua7hxU8EXEA=
github.com/pion/dtls v1.3.4/go.mod h1:CjlPLfQdsTg3G4AEXjJp8FY5bRweBlxHrgoFrN+fQsk=
github.com/pion/ice v0.2.6 h1:N/xhQtO6WfWlyMvIZgE+cqG5AHJnKL+aEboade72qno=
github.com/pion/ice v0.2.6/go.mod h1:igvbO76UeYthbSu0UsUTqjyWpFT3diUmM+x2vt4p4fw=
github.com/pion/ice v0.2.7 h1:dmjqaGoC+/QIOv9j9lHnFmaECm2YOir25ivLbSN4kF4=
github.com/pion/ice v0.2.7/go.mod h1:igvbO76UeYthbSu0UsUTqjyWpFT3diUmM+x2vt4p4fw=
github.com/pion/logging v0.2.1 h1:LwASkBKZ+2ysGJ+jLv1E/9H1ge0k1nTfi1X+5zirkDk=
github.com/pion/logging v0.2.1/go.mod h1:k0/tDVsRCX2Mb2ZEmTqNa7CWsQPc+YYCB7Q+5pahoms=
github.com/pion/quic v0.1.1 h1:D951FV+TOqI9A0rTF7tHx0Loooqz+nyzjEyj8o3PuMA=
Expand Down
26 changes: 23 additions & 3 deletions icetransport.go
Expand Up @@ -25,6 +25,8 @@ type ICETransport struct {
onConnectionStateChangeHdlr func(ICETransportState)
onSelectedCandidatePairChangeHdlr func(*ICECandidatePair)

state ICETransportState

gatherer *ICEGatherer
conn *ice.Conn
mux *mux.Mux
Expand Down Expand Up @@ -62,6 +64,7 @@ func (api *API) NewICETransport(gatherer *ICEGatherer) *ICETransport {
gatherer: gatherer,
api: api,
log: api.settingEngine.LoggerFactory.NewLogger("ortc"),
state: ICETransportStateNew,
}
}

Expand All @@ -80,7 +83,12 @@ func (t *ICETransport) Start(gatherer *ICEGatherer, params ICEParameters, role *

agent := t.gatherer.agent
if err := agent.OnConnectionStateChange(func(iceState ice.ConnectionState) {
t.onConnectionStateChange(newICETransportStateFromICE(iceState))
state := newICETransportStateFromICE(iceState)
t.lock.Lock()
t.state = state
t.lock.Unlock()

t.onConnectionStateChange(state)
}); err != nil {
return err
}
Expand Down Expand Up @@ -142,7 +150,6 @@ func (t *ICETransport) Start(gatherer *ICEGatherer, params ICEParameters, role *

// Stop irreversibly stops the ICETransport.
func (t *ICETransport) Stop() error {
// Close the Mux. This closes the Mux and the underlying ICE conn.
t.lock.Lock()
defer t.lock.Unlock()

Expand All @@ -151,7 +158,6 @@ func (t *ICETransport) Stop() error {
} else if t.gatherer != nil {
return t.gatherer.Close()
}

return nil
}

Expand Down Expand Up @@ -241,6 +247,20 @@ func (t *ICETransport) AddRemoteCandidate(remoteCandidate ICECandidate) error {
return nil
}

// State returns the current ice transport state.
func (t *ICETransport) State() ICETransportState {
t.lock.RLock()
defer t.lock.RUnlock()
return t.state
}

// NewEndpoint registers a new endpoint on the underlying mux.
func (t *ICETransport) NewEndpoint(f mux.MatchFunc) *mux.Endpoint {
t.lock.Lock()
defer t.lock.Unlock()
return t.mux.NewEndpoint(f)
}

func (t *ICETransport) ensureGatherer() error {
if t.gatherer == nil ||
t.gatherer.agent == nil {
Expand Down
16 changes: 9 additions & 7 deletions peerconnection_close_test.go
Expand Up @@ -86,22 +86,24 @@ func TestPeerConnection_Close_PreICE(t *testing.T) {
t.Fatal(err)
}

for {
if pcAnswer.iceTransport.State() == ICETransportStateChecking {
break
}
time.Sleep(time.Second)
}

err = pcAnswer.Close()
if err != nil {
t.Fatal(err)
}

// Assert that ICEGatherer is shutdown, test timeout will prevent deadlock
pcAnswer.iceTransport.lock.Lock()
gatherer := pcAnswer.iceTransport.gatherer
pcAnswer.iceTransport.lock.Unlock()
// Assert that ICETransport is shutdown, test timeout will prevent deadlock
for {
gatherer.lock.RLock()
if gatherer.agent == nil {
if pcAnswer.iceTransport.State() == ICETransportStateClosed {
time.Sleep(time.Second * 3)
return
}
gatherer.lock.RUnlock()

time.Sleep(time.Second)
}
Expand Down
5 changes: 2 additions & 3 deletions quictransport.go
Expand Up @@ -118,7 +118,7 @@ func (t *QUICTransport) Start(remoteParameters QUICParameters) error {
Certificate: cert.x509Cert,
PrivateKey: cert.privateKey,
}
endpoint := t.iceTransport.mux.NewEndpoint(mux.MatchAll)
endpoint := t.iceTransport.NewEndpoint(mux.MatchAll)
err := t.TransportBase.StartBase(endpoint, cfg)
if err != nil {
return err
Expand Down Expand Up @@ -161,8 +161,7 @@ func (t *QUICTransport) validateFingerPrint(remoteParameters QUICParameters, rem

func (t *QUICTransport) ensureICEConn() error {
if t.iceTransport == nil ||
t.iceTransport.conn == nil ||
t.iceTransport.mux == nil {
t.iceTransport.State() == ICETransportStateNew {
return errors.New("ICE connection not started")
}

Expand Down

0 comments on commit 736e0bb

Please sign in to comment.