Skip to content

Commit

Permalink
Marco/with after func (#763)
Browse files Browse the repository at this point in the history
refactor: if the conn has a context, use context.AfterFunc for cleanup
---------

Co-authored-by: Marco Munizaga <[email protected]>
  • Loading branch information
gammazero and MarcoPolo committed Dec 18, 2024
1 parent de6a1ed commit af8522d
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ The following emojis are used to highlight certain changes:
- 🛠 `blockstore` and `blockservice`'s `WriteThrough()` option now takes an "enabled" parameter: `WriteThrough(enabled bool)`.
- Replaced unmaintained mock time implementation uses in tests: [from](github.com/benbjohnson/clock) => [to](github.com/filecoin-project/go-clock)
- updated to go-libp2p to [v0.38.0](https://github.com/libp2p/go-libp2p/releases/tag/v0.38.0)
- `bitswap/client`: if a libp2p connection has a context, use `context.AfterFunc` to cleanup the connection.


### Removed
Expand Down
44 changes: 36 additions & 8 deletions bitswap/network/ipfs_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,32 @@ type impl struct {
receivers []Receiver
}

// interfaceWrapper is concrete type that wraps an interface. Necessary because
// atomic.Value needs the same type and can not Store(nil). This indirection
// allows us to store nil.
type interfaceWrapper[T any] struct {
t T
}
type atomicInterface[T any] struct {
iface atomic.Value
}

func (a *atomicInterface[T]) Load() T {
var v T
x := a.iface.Load()
if x != nil {
return x.(interfaceWrapper[T]).t
}
return v
}

func (a *atomicInterface[T]) Store(v T) {
a.iface.Store(interfaceWrapper[T]{v})
}

type streamMessageSender struct {
to peer.ID
stream network.Stream
stream atomicInterface[network.Stream]
bsnet *impl
opts *MessageSenderOpts
}
Expand All @@ -95,7 +118,7 @@ type HasContext interface {

// Open a stream to the remote peer
func (s *streamMessageSender) Connect(ctx context.Context) (network.Stream, error) {
stream := s.stream
stream := s.stream.Load()
if stream != nil {
return stream, nil
}
Expand All @@ -111,36 +134,41 @@ func (s *streamMessageSender) Connect(ctx context.Context) (network.Stream, erro
if err != nil {
return nil, err
}
if withCtx, ok := stream.Conn().(HasContext); ok {
context.AfterFunc(withCtx.Context(), func() {
s.stream.Store(nil)
})

Check warning on line 140 in bitswap/network/ipfs_impl.go

View check run for this annotation

Codecov / codecov/patch

bitswap/network/ipfs_impl.go#L138-L140

Added lines #L138 - L140 were not covered by tests
}

s.stream = stream
s.stream.Store(stream)
return stream, nil
}

// Reset the stream
func (s *streamMessageSender) Reset() error {
stream := s.stream
stream := s.stream.Load()
if stream != nil {
err := stream.Reset()
s.stream = nil
s.stream.Store(nil)
return err
}
return nil
}

// Close the stream
func (s *streamMessageSender) Close() error {
stream := s.stream
stream := s.stream.Load()
if stream != nil {
err := stream.Close()
s.stream = nil
s.stream.Store(nil)
return err
}
return nil
}

// Indicates whether the peer supports HAVE / DONT_HAVE messages
func (s *streamMessageSender) SupportsHave() bool {
stream := s.stream
stream := s.stream.Load()
if stream == nil {
return false
}

Check warning on line 174 in bitswap/network/ipfs_impl.go

View check run for this annotation

Codecov / codecov/patch

bitswap/network/ipfs_impl.go#L173-L174

Added lines #L173 - L174 were not covered by tests
Expand Down

0 comments on commit af8522d

Please sign in to comment.