Skip to content

Commit fe11efd

Browse files
committed
Fix race condition and wonky logs
This commit fixes a race condition when sharing state with multiple peers at once. It also fixes logs and errors using `%q` as a formatting directive for a `uint64` which ends up coming out in the form `\x0`. Signed-off-by: David Bond <davidsbond93@gmail.com>
1 parent 0e3a2eb commit fe11efd

File tree

1 file changed

+27
-19
lines changed

1 file changed

+27
-19
lines changed

whisper.go

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -687,25 +687,33 @@ func (n *Node) shareState(ctx context.Context) error {
687687
var group errgroup.Group
688688
for _, target := range targets {
689689
group.Go(func() error {
690-
for _, p := range peers {
691-
if p.ID == target.ID {
692-
// Don't tell peers about themselves, each peer owns its own state except in the scenario where
693-
// one is leaving. But if it's leaving, it won't care for more updates.
694-
continue
695-
}
696-
697-
if err = n.sendPeerMessage(target, p); err != nil {
698-
return fmt.Errorf("failed to send peer message to peer %q: %w", target.ID, err)
699-
}
700-
}
701-
702-
return nil
690+
return n.sendPeerMessages(ctx, target, peers)
703691
})
704692
}
705693

706694
return group.Wait()
707695
}
708696

697+
func (n *Node) sendPeerMessages(ctx context.Context, target peer.Peer, peers []peer.Peer) error {
698+
for _, p := range peers {
699+
if ctx.Err() != nil {
700+
return ctx.Err()
701+
}
702+
703+
if p.ID == target.ID {
704+
// Don't tell peers about themselves, each peer owns its own state except in the scenario where
705+
// one is leaving. But if it's leaving, it won't care for more updates.
706+
continue
707+
}
708+
709+
if err := n.sendPeerMessage(target, p); err != nil {
710+
return fmt.Errorf("failed to send peer message to peer %d: %w", target.ID, err)
711+
}
712+
}
713+
714+
return nil
715+
}
716+
709717
func (n *Node) checkPeer(ctx context.Context) error {
710718
target, err := n.selectPeer(ctx)
711719
if err != nil {
@@ -719,14 +727,14 @@ func (n *Node) checkPeer(ctx context.Context) error {
719727
client, closer, err := peer.Dial(target.Address, n.clientTLS)
720728
if err != nil {
721729
if err = n.checkPeerViaPeers(ctx, target); err != nil {
722-
return fmt.Errorf("failed to check peer %q via peer: %w", target.ID, err)
730+
return fmt.Errorf("failed to check peer %d via peer: %w", target.ID, err)
723731
}
724732
}
725733

726734
defer closer()
727735
if _, err = client.Status(ctx, &whispersvcv1.StatusRequest{}); err != nil {
728736
if err = n.checkPeerViaPeers(ctx, target); err != nil {
729-
return fmt.Errorf("failed to check peer %q via peer: %w", target.ID, err)
737+
return fmt.Errorf("failed to check peer %d via peer: %w", target.ID, err)
730738
}
731739
}
732740

@@ -749,7 +757,7 @@ func (n *Node) checkPeerViaPeers(ctx context.Context, target peer.Peer) error {
749757
for _, via := range checkVia {
750758
reached, err := n.checkPeerViaPeer(ctx, target, via)
751759
if err != nil {
752-
return fmt.Errorf("failed to check peer %q via peer %q: %w", target.ID, via.ID, err)
760+
return fmt.Errorf("failed to check peer %d via peer %d: %w", target.ID, via.ID, err)
753761
}
754762

755763
// At least one of our selected peers was able to reach the target peer. Or they had an entry for that peer
@@ -764,7 +772,7 @@ func (n *Node) checkPeerViaPeers(ctx context.Context, target peer.Peer) error {
764772
WarnContext(ctx, "peer was unavailable via other peers, marking as gone")
765773

766774
if err = n.markPeerGone(ctx, target); err != nil {
767-
return fmt.Errorf("failed to mark peer %q as gone: %w", target.ID, err)
775+
return fmt.Errorf("failed to mark peer %d as gone: %w", target.ID, err)
768776
}
769777

770778
if err = n.writeEvent(ctx, event.TypeGone, target); err != nil {
@@ -799,7 +807,7 @@ func (n *Node) checkPeerViaPeer(ctx context.Context, target peer.Peer, via peer.
799807
return false, nil
800808
default:
801809
// All other status codes are unexpected. We report that.
802-
return false, fmt.Errorf("unexpected error checking peer %q: %w", target.ID, err)
810+
return false, fmt.Errorf("unexpected error checking peer %d: %w", target.ID, err)
803811
}
804812
}
805813

@@ -889,7 +897,7 @@ func (n *Node) reap(ctx context.Context) error {
889897
case errors.Is(err, store.ErrPeerNotFound):
890898
continue
891899
case err != nil:
892-
return fmt.Errorf("failed to remove peer %q: %w", p.ID, err)
900+
return fmt.Errorf("failed to remove peer %d: %w", p.ID, err)
893901
}
894902

895903
n.ciphers.Remove(p.Hash())

0 commit comments

Comments
 (0)