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

relaymanager: do not start new relay if one already exists #2093

Merged
merged 5 commits into from
Feb 16, 2023
Merged

relaymanager: do not start new relay if one already exists #2093

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e5442ec
do not start new relay if one already exists
sukunrt Feb 14, 2023
cacb452
Update p2p/host/relaysvc/relay.go
sukunrt Feb 15, 2023
6be1bd4
test by emitting events
sukunrt Feb 15, 2023
b5bca4f
fix race condition
sukunrt Feb 15, 2023
80e8cc3
increase timeout
sukunrt Feb 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions p2p/host/relaysvc/relay.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,18 @@ func (m *RelayManager) background(ctx context.Context) {
func (m *RelayManager) reachabilityChanged(r network.Reachability) error {
switch r {
case network.ReachabilityPublic:
m.mutex.Lock()
defer m.mutex.Unlock()
// ignore events with no reachability change
sukunrt marked this conversation as resolved.
Show resolved Hide resolved
if m.relay != nil {
return nil
}
relay, err := relayv2.New(m.host, m.opts...)
if err != nil {
return err
}
m.mutex.Lock()
defer m.mutex.Unlock()
m.relay = relay
case network.ReachabilityPrivate:
default:
m.mutex.Lock()
defer m.mutex.Unlock()
if m.relay != nil {
Expand Down
29 changes: 29 additions & 0 deletions p2p/host/relaysvc/relay_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package relaysvc

import (
"testing"

"github.com/libp2p/go-libp2p/core/network"
bhost "github.com/libp2p/go-libp2p/p2p/host/blank"
swarmt "github.com/libp2p/go-libp2p/p2p/net/swarm/testing"
"github.com/stretchr/testify/require"
)

func TestReachabilityChangeEvent(t *testing.T) {
h := bhost.NewBlankHost(swarmt.GenSwarm(t))
rmgr := NewRelayManager(h)
rmgr.reachabilityChanged(network.ReachabilityPublic)
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a slight preference for not calling unexported functions in tests (where possible). In this case, you could get the event bus (via h.EventBus()), and then emit reachability events.

Copy link
Member Author

Choose a reason for hiding this comment

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

that's neat!

Copy link
Member Author

Choose a reason for hiding this comment

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

this is fixed now

require.NotNil(t, rmgr.relay, "relay should be set on public reachability")

rmgr.reachabilityChanged(network.ReachabilityPrivate)
require.Nil(t, rmgr.relay, "relay should be nil on private reachability")

rmgr.reachabilityChanged(network.ReachabilityPublic)
rmgr.reachabilityChanged(network.ReachabilityUnknown)
require.Nil(t, rmgr.relay, "relay should be nil on unknown reachability")

rmgr.reachabilityChanged(network.ReachabilityPublic)
relay := rmgr.relay
rmgr.reachabilityChanged(network.ReachabilityPublic)
require.Equal(t, relay, rmgr.relay, "relay should not be started on receiving the same event")
}