Skip to content

Commit

Permalink
fix(network): fix inconsistent typed value issue in DelegatePacketPro…
Browse files Browse the repository at this point in the history
…xy (#35)

`atomic.Value` doesn't accept values with different `struct` types (even if you cast the value to the same interface). So I use `atomic.Pointer[]` instead.
  • Loading branch information
jyyi1 committed Aug 7, 2023
1 parent da98e16 commit 685046f
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 4 deletions.
11 changes: 7 additions & 4 deletions network/delegate_packet_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ var errInvalidProxy = errors.New("the underlying proxy must not be nil")
var _ DelegatePacketProxy = (*delegatePacketProxy)(nil)

type delegatePacketProxy struct {
proxy atomic.Value
// The underlying PacketProxy when create NewSession.
// Note that we must not use atomic.Value; otherwise TestSetProxyOfDifferentTypes will panic with
// "store inconsistently typed value".
proxy atomic.Pointer[PacketProxy]
}

// NewDelegatePacketProxy creates a new [DelegatePacketProxy] that forwards calls to the `proxy` [PacketProxy].
Expand All @@ -51,20 +54,20 @@ func NewDelegatePacketProxy(proxy PacketProxy) (DelegatePacketProxy, error) {
return nil, errInvalidProxy
}
dp := delegatePacketProxy{}
dp.proxy.Store(proxy)
dp.proxy.Store(&proxy)
return &dp, nil
}

// NewSession implements PacketProxy.NewSession, and it will forward the call to the underlying PacketProxy.
func (p *delegatePacketProxy) NewSession(respWriter PacketResponseReceiver) (PacketRequestSender, error) {
return p.proxy.Load().(PacketProxy).NewSession(respWriter)
return (*p.proxy.Load()).NewSession(respWriter)
}

// SetProxy implements DelegatePacketProxy.SetProxy.
func (p *delegatePacketProxy) SetProxy(proxy PacketProxy) error {
if proxy == nil {
return errInvalidProxy
}
p.proxy.Store(proxy)
p.proxy.Store(&proxy)
return nil
}
27 changes: 27 additions & 0 deletions network/delegate_packet_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,26 @@ func TestSetProxyWithNilValue(t *testing.T) {
require.Error(t, err)
}

// Make sure we can SetProxy to different types
func TestSetProxyOfDifferentTypes(t *testing.T) {
defProxy := &sessionCountPacketProxy{}
newProxy := &noopPacketProxy{}

p, err := NewDelegatePacketProxy(defProxy)
require.NotNil(t, p)
require.NoError(t, err)

// SetProxy should not return error
err = p.SetProxy(newProxy)
require.NoError(t, err)

// NewSession' should not go to defProxy
snd, err := p.NewSession(nil)
require.Nil(t, snd)
require.NoError(t, err)
require.Exactly(t, 0, defProxy.Count())
}

// sessionCountPacketProxy logs the count of the NewSession calls, and returns a nil PacketRequestSender
type sessionCountPacketProxy struct {
cnt atomic.Int32
Expand All @@ -138,3 +158,10 @@ func (sp *sessionCountPacketProxy) NewSession(respWriter PacketResponseReceiver)
func (sp *sessionCountPacketProxy) Count() int {
return int(sp.cnt.Load())
}

type noopPacketProxy struct {
}

func (noopPacketProxy) NewSession(PacketResponseReceiver) (PacketRequestSender, error) {
return nil, nil
}

0 comments on commit 685046f

Please sign in to comment.