Skip to content

Commit

Permalink
fix 'Segmentation violation in nsmgr' (#1642)
Browse files Browse the repository at this point in the history
* fix 'Segmentation violation in nsmgr'

Signed-off-by: denis-tingaikin <denis.tingajkin@xored.com>

* add unit test to cover issue

Signed-off-by: denis-tingaikin <denis.tingajkin@xored.com>

* Fix linter issue

Signed-off-by: denis-tingaikin <denis.tingajkin@xored.com>

---------

Signed-off-by: denis-tingaikin <denis.tingajkin@xored.com>
  • Loading branch information
denis-tingaikin authored Jul 3, 2024
1 parent 73a3c05 commit 21369bd
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 2 deletions.
8 changes: 6 additions & 2 deletions pkg/registry/common/recvfd/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (r *recvfdNseServer) Unregister(ctx context.Context, endpoint *registry.Net

// Get the grpcfd.FDRecver
recv, ok := grpcfd.FromContext(ctx)
if !ok {
if !ok || recv == nil {
return next.NetworkServiceEndpointRegistryServer(ctx).Unregister(ctx, endpoint)
}
// Get the fileMap
Expand Down Expand Up @@ -168,7 +168,11 @@ func recvFDAndSwapInodeToUnix(ctx context.Context, fileMap *perEndpointFileMap,
case <-ctx.Done():
err = errors.Wrap(ctx.Err(), "recvFDAndSwapInodeToUnix context is done")
return
case file = <-fileCh:
case file, ok = <-fileCh:
if !ok {
err = errors.New("files channel was closed")
return
}
// If we get the file, remember it in the fileMap so we can reuse it later
// Note: This is done because we want to present a single consistent filename to
// any of the other chain elements using the information, and since that filename will be
Expand Down
61 changes: 61 additions & 0 deletions pkg/registry/common/recvfd/server_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Copyright (c) 2021-2022 Doc.ai and/or its affiliates.
//
// Copyright (c) 2024 Cisco and/or its affiliates.
//
// SPDX-License-Identifier: Apache-2.0
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand All @@ -21,6 +23,7 @@ package recvfd_test

import (
"context"
"net"
"net/url"
"os"
"path"
Expand All @@ -31,8 +34,10 @@ import (
"github.com/edwarnicke/grpcfd"
"github.com/networkservicemesh/api/pkg/api/registry"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/peer"

registryserver "github.com/networkservicemesh/sdk/pkg/registry"
"github.com/networkservicemesh/sdk/pkg/registry/common/begin"
Expand Down Expand Up @@ -156,3 +161,59 @@ func TestNseRecvfdServerClosesFile(t *testing.T) {
return fileClosedContext.Err() != nil
}, time.Second, time.Millisecond*100)
}

type myFDTransceiver struct {
net.Addr
ch chan *os.File
}

func (f *myFDTransceiver) RecvFD(dev, inode uint64) <-chan uintptr {
return nil
}
func (f *myFDTransceiver) RecvFile(dev, ino uint64) <-chan *os.File {
return nil
}
func (f *myFDTransceiver) RecvFileByURL(urlStr string) (<-chan *os.File, error) {
return f.ch, nil
}

func (f *myFDTransceiver) SendFD(fd uintptr) <-chan error {
return nil
}
func (f *myFDTransceiver) SendFile(file grpcfd.SyscallConn) <-chan error {
return nil
}
func (f *myFDTransceiver) SendFilename(filename string) <-chan error {
return nil
}

var _ grpcfd.FDTransceiver = (*myFDTransceiver)(nil)

func (f *myFDTransceiver) RecvFDByURL(urlStr string) (<-chan uintptr, error) {
return nil, nil
}

func TestNseRecvfdServer_ClosesFile(t *testing.T) {
t.Cleanup(func() {
goleak.VerifyNone(t)
})

ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
f := myFDTransceiver{ch: make(chan *os.File)}
p := peer.Peer{
Addr: &f,
}
s := registryrecvfd.NewNetworkServiceEndpointRegistryServer()

defer cancel()

go func() {
time.Sleep(time.Second / 2)
close(f.ch)
}()

ctx = peer.NewContext(ctx, &p)

_, _ = s.Register(ctx, &registry.NetworkServiceEndpoint{Name: "test", Url: "inode://proc/1/fd/1"})
_, _ = s.Unregister(ctx, &registry.NetworkServiceEndpoint{Name: "test", Url: "inode://proc/1/fd/1"})
}

0 comments on commit 21369bd

Please sign in to comment.