Skip to content

Commit

Permalink
Addressed review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Oleg Solodkov <oleg.solodkov@xored.com>
  • Loading branch information
sol-0 committed Dec 3, 2021
1 parent 0c19391 commit 55d55c8
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 21 deletions.
23 changes: 16 additions & 7 deletions pkg/networkservice/common/mechanisms/recvfd/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import (
"github.com/networkservicemesh/api/pkg/api/networkservice/mechanisms/cls"
"github.com/networkservicemesh/api/pkg/api/networkservice/mechanisms/common"
"github.com/networkservicemesh/api/pkg/api/networkservice/mechanisms/kernel"
"github.com/networkservicemesh/sdk/pkg/tools/grpcfdutils"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
"google.golang.org/grpc"
Expand All @@ -46,14 +45,14 @@ import (
"github.com/networkservicemesh/sdk/pkg/networkservice/common/mechanisms/sendfd"
"github.com/networkservicemesh/sdk/pkg/networkservice/core/chain"
"github.com/networkservicemesh/sdk/pkg/networkservice/core/next"
"github.com/networkservicemesh/sdk/pkg/tools/grpcfdutils"
"github.com/networkservicemesh/sdk/pkg/tools/grpcutils"
"github.com/networkservicemesh/sdk/pkg/tools/sandbox"
)

type checkRecvfdServer struct {
onRecvFile map[string]func()

t *testing.T
t *testing.T
onFileClosedCallbacks map[string]func()
}

func (n *checkRecvfdServer) Close(ctx context.Context, conn *networkservice.Connection) (*empty.Empty, error) {
Expand All @@ -76,6 +75,16 @@ func (n *checkRecvfdServer) Request(ctx context.Context, request *networkservice
return next.Server(ctx).Request(ctx, request)
}

func (n *checkRecvfdServer) onRecvFile(fileName string, file *os.File) {
// checking a file is closed by recvfd by setting a finalizer
runtime.SetFinalizer(file, func(file *os.File) {
onFileClosedCallback, ok := n.onFileClosedCallbacks[fileName]
if ok {
onFileClosedCallback()
}
})
}

func createFile(fileName string, t *testing.T) (inodeURLStr string, fileClosedContext context.Context, cancelFunc func()) {
f, err := os.Create(fileName)
require.NoError(t, err, "Failed to create and open a file: %v", err)
Expand Down Expand Up @@ -134,7 +143,7 @@ func TestRecvfdClosesSingleFile(t *testing.T) {
var testChain = chain.NewNetworkServiceServer(
&checkRecvfdServer{
t: t,
onRecvFile: map[string]func(){
onFileClosedCallbacks: map[string]func(){
inodeURLStr: cancelFunc,
},
},
Expand Down Expand Up @@ -208,8 +217,8 @@ func TestRecvfdClosesMultipleFiles(t *testing.T) {

var testChain = chain.NewNetworkServiceServer(
&checkRecvfdServer{
t: t,
onRecvFile: onFileClosedFuncs,
t: t,
onFileClosedCallbacks: onFileClosedFuncs,
},
recvfd.NewServer())

Expand Down
21 changes: 15 additions & 6 deletions pkg/registry/common/recvfd/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package recvfd_test
import (
"context"
"net/url"
"os"
"path"
"runtime"
"testing"
Expand Down Expand Up @@ -50,9 +51,8 @@ import (
)

type checkNseRecvfdServer struct {
onRecvFile map[string]func()

t *testing.T
t *testing.T
onFileClosedCallbacks map[string]func()
}

func (n *checkNseRecvfdServer) Unregister(ctx context.Context, endpoint *registry.NetworkServiceEndpoint) (*empty.Empty, error) {
Expand All @@ -79,6 +79,15 @@ func (n *checkNseRecvfdServer) Find(query *registry.NetworkServiceEndpointQuery,
return next.NetworkServiceEndpointRegistryServer(server.Context()).Find(query, server)
}

func (n *checkNseRecvfdServer) onRecvFile(fileName string, file *os.File) {
runtime.SetFinalizer(file, func(file *os.File) {
onFileClosedCallback, ok := n.onFileClosedCallbacks[fileName]
if ok {
onFileClosedCallback()
}
})
}

func getFileInfo(fileName string, t *testing.T) (inodeURLStr string, fileClosedContext context.Context, cancelFunc func()) {
fileClosedContext, cancelFunc = context.WithCancel(context.Background())

Expand Down Expand Up @@ -109,8 +118,8 @@ func TestNseRecvfdServerClosesFile(t *testing.T) {
)

var checkRecvfdServer = &checkNseRecvfdServer{
t: t,
onRecvFile: make(map[string]func()),
t: t,
onFileClosedCallbacks: make(map[string]func()),
}

var nseRegistry = registrychain.NewNetworkServiceEndpointRegistryServer(
Expand Down Expand Up @@ -151,7 +160,7 @@ func TestNseRecvfdServerClosesFile(t *testing.T) {
// setting onRecvFile after starting the server as we're re-creating a socket in grpcutils.ListenAndServe
// in registry case we're passing a socket
inodeURLStr, fileClosedContext, cancelFunc := getFileInfo(testFileName, t)
checkRecvfdServer.onRecvFile[inodeURLStr] = cancelFunc
checkRecvfdServer.onFileClosedCallbacks[inodeURLStr] = cancelFunc

var testEndpoint = &registry.NetworkServiceEndpoint{
Name: "test-endpoint",
Expand Down
16 changes: 8 additions & 8 deletions pkg/tools/grpcfdutils/transceiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,28 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// +build linux

// Package grpcfdutils provides utilities for grpcfd library
package grpcfdutils

import (
"net"
"os"
"runtime"

"github.com/edwarnicke/grpcfd"
)

// NotifiableFDTransceiver - grpcfd.Transceiver wrapper checking that received FDs are closed
// OnRecvFile - callback receiving inodeURL string and a file received by grpcfd
type NotifiableFDTransceiver struct {
grpcfd.FDTransceiver
net.Addr

OnRecvFile map[string]func()
OnRecvFile func(string, *os.File)
}

// RecvFileByURL - wrapper of grpcfd.FDRecver method invoking callback when a file is received by grpcfd
func (w *NotifiableFDTransceiver) RecvFileByURL(urlStr string) (<-chan *os.File, error) {
recv, err := w.FDTransceiver.RecvFileByURL(urlStr)
if err != nil {
Expand All @@ -40,12 +45,7 @@ func (w *NotifiableFDTransceiver) RecvFileByURL(urlStr string) (<-chan *os.File,
var fileCh = make(chan *os.File)
go func() {
for f := range recv {
runtime.SetFinalizer(f, func(file *os.File) {
onFileClosedFunc, ok := w.OnRecvFile[urlStr]
if ok {
onFileClosedFunc()
}
})
w.OnRecvFile(urlStr, f)
fileCh <- f
}
}()
Expand Down

0 comments on commit 55d55c8

Please sign in to comment.