From 21369bd95508ec676df557d36a6e6d16367a6113 Mon Sep 17 00:00:00 2001 From: Denis Tingaikin Date: Wed, 3 Jul 2024 17:04:24 +0300 Subject: [PATCH] fix 'Segmentation violation in nsmgr' (#1642) * fix 'Segmentation violation in nsmgr' Signed-off-by: denis-tingaikin * add unit test to cover issue Signed-off-by: denis-tingaikin * Fix linter issue Signed-off-by: denis-tingaikin --------- Signed-off-by: denis-tingaikin --- pkg/registry/common/recvfd/server.go | 8 ++- pkg/registry/common/recvfd/server_test.go | 61 +++++++++++++++++++++++ 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/pkg/registry/common/recvfd/server.go b/pkg/registry/common/recvfd/server.go index 7d06fcd21..01c795988 100644 --- a/pkg/registry/common/recvfd/server.go +++ b/pkg/registry/common/recvfd/server.go @@ -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 @@ -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 diff --git a/pkg/registry/common/recvfd/server_test.go b/pkg/registry/common/recvfd/server_test.go index 8e316cc46..78427cc64 100644 --- a/pkg/registry/common/recvfd/server_test.go +++ b/pkg/registry/common/recvfd/server_test.go @@ -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"); @@ -21,6 +23,7 @@ package recvfd_test import ( "context" + "net" "net/url" "os" "path" @@ -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" @@ -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, ®istry.NetworkServiceEndpoint{Name: "test", Url: "inode://proc/1/fd/1"}) + _, _ = s.Unregister(ctx, ®istry.NetworkServiceEndpoint{Name: "test", Url: "inode://proc/1/fd/1"}) +}