diff --git a/pkg/networkservice/common/mechanisms/recvfd/client.go b/pkg/networkservice/common/mechanisms/recvfd/client.go index 09cd360a5..70e2815dd 100644 --- a/pkg/networkservice/common/mechanisms/recvfd/client.go +++ b/pkg/networkservice/common/mechanisms/recvfd/client.go @@ -2,6 +2,8 @@ // // Copyright (c) 2020-2023 Cisco and/or its affiliates. // +// Copyright (c) 2024 Xored Software Inc and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -68,6 +70,7 @@ func (r *recvFDClient) Request(ctx context.Context, request *networkservice.Netw // Recv the FD and swap theInode to File in the Parameters for the returned connection mechanism err = recvFDAndSwapInodeToFile(ctx, fileMap, conn.GetMechanism().GetParameters(), recv) if err != nil { + closeFiles(conn, &r.fileMaps) return nil, err } diff --git a/pkg/networkservice/common/mechanisms/recvfd/server.go b/pkg/networkservice/common/mechanisms/recvfd/server.go index 6846f488c..201a37745 100644 --- a/pkg/networkservice/common/mechanisms/recvfd/server.go +++ b/pkg/networkservice/common/mechanisms/recvfd/server.go @@ -1,5 +1,7 @@ // Copyright (c) 2020-2023 Cisco and/or its affiliates. // +// Copyright (c) 2024 Xored Software Inc and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -59,6 +61,7 @@ func (r *recvFDServer) Request(ctx context.Context, request *networkservice.Netw for _, mechanism := range append(request.GetMechanismPreferences(), request.GetConnection().GetMechanism()) { err := recvFDAndSwapInodeToFile(ctx, fileMap, mechanism.GetParameters(), recv) if err != nil { + closeFiles(request.GetConnection(), &r.fileMaps) return nil, err } } @@ -66,12 +69,14 @@ func (r *recvFDServer) Request(ctx context.Context, request *networkservice.Netw // Call the next server in the chain conn, err := next.Server(ctx).Request(ctx, request) if err != nil { + closeFiles(request.GetConnection(), &r.fileMaps) return nil, err } // Swap back from File to Inode in the InodeURL in the Parameters err = swapFileToInode(fileMap, conn.GetMechanism().GetParameters()) if err != nil { + closeFiles(request.GetConnection(), &r.fileMaps) return nil, err } @@ -80,7 +85,7 @@ func (r *recvFDServer) Request(ctx context.Context, request *networkservice.Netw func (r *recvFDServer) Close(ctx context.Context, conn *networkservice.Connection) (*empty.Empty, error) { // Clean up the fileMap no matter what happens - defer r.closeFiles(conn) + defer closeFiles(conn, &r.fileMaps) // Get the grpcfd.FDRecver recv, ok := grpcfd.FromContext(ctx) @@ -111,16 +116,14 @@ func (r *recvFDServer) Close(ctx context.Context, conn *networkservice.Connectio return &empty.Empty{}, err } -func (r *recvFDServer) closeFiles(conn *networkservice.Connection) { - defer r.fileMaps.Delete(conn.GetId()) - - fileMap, _ := r.fileMaps.LoadOrStore(conn.GetId(), &perConnectionFileMap{ - filesByInodeURL: make(map[string]*os.File), - inodeURLbyFilename: make(map[string]*url.URL), - }) - +func closeFiles(conn *networkservice.Connection, fileMaps *genericsync.Map[string, *perConnectionFileMap]) { + fileMap, loaded := fileMaps.LoadAndDelete(conn.GetId()) + if !loaded { + return + } for inodeURLStr, file := range fileMap.filesByInodeURL { delete(fileMap.filesByInodeURL, inodeURLStr) _ = file.Close() + _ = os.Remove(file.Name()) } } diff --git a/pkg/registry/common/recvfd/client.go b/pkg/registry/common/recvfd/client.go index ec76a276d..c6f00e5a0 100644 --- a/pkg/registry/common/recvfd/client.go +++ b/pkg/registry/common/recvfd/client.go @@ -2,6 +2,8 @@ // // Copyright (c) 2023 Cisco and/or its affiliates. // +// Copyright (c) 2024 Xored Software Inc and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -99,6 +101,9 @@ func (x *recvfdNSEFindClient) Recv() (*registry.NetworkServiceEndpointResponse, // Recv the FD and swap theInode to File in the Parameters for the returned connection mechanism err = recvFDAndSwapInodeToUnix(x.Context(), fileMap, nseResp.GetNetworkServiceEndpoint(), x.transceiver) + if err != nil { + closeFiles(nseResp.GetNetworkServiceEndpoint(), x.fileMaps) + } } return nseResp, err } diff --git a/pkg/registry/common/recvfd/server.go b/pkg/registry/common/recvfd/server.go index 2590e2d88..7d06fcd21 100644 --- a/pkg/registry/common/recvfd/server.go +++ b/pkg/registry/common/recvfd/server.go @@ -1,5 +1,7 @@ // Copyright (c) 2020-2023 Cisco and/or its affiliates. // +// Copyright (c) 2024 Xored Software Inc and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -70,12 +72,14 @@ func (r *recvfdNseServer) Register(ctx context.Context, endpoint *registry.Netwo endpoint = endpoint.Clone() err := recvFDAndSwapInodeToUnix(ctx, fileMap, endpoint, recv) if err != nil { + closeFiles(endpoint, &r.fileMaps) return nil, err } // Call the next server in the chain returnedEndpoint, err := next.NetworkServiceEndpointRegistryServer(ctx).Register(ctx, endpoint) if err != nil { + closeFiles(endpoint, &r.fileMaps) return nil, err } returnedEndpoint = returnedEndpoint.Clone() @@ -87,6 +91,7 @@ func (r *recvfdNseServer) Register(ctx context.Context, endpoint *registry.Netwo // Swap back from File to Inode in the InodeURL in the Parameters err = swapFileToInode(fileMap, returnedEndpoint) if err != nil { + closeFiles(endpoint, &r.fileMaps) return nil, err } return returnedEndpoint, nil @@ -102,7 +107,7 @@ func (r *recvfdNseServer) Unregister(ctx context.Context, endpoint *registry.Net } // Clean up the fileMap no matter what happens - defer r.closeFiles(endpoint) + defer closeFiles(endpoint, &r.fileMaps) // Get the grpcfd.FDRecver recv, ok := grpcfd.FromContext(ctx) @@ -214,16 +219,17 @@ func swapFileToInode(fileMap *perEndpointFileMap, endpoint *registry.NetworkServ return nil } -func (r *recvfdNseServer) closeFiles(endpoint *registry.NetworkServiceEndpoint) { - defer r.fileMaps.Delete(endpoint.GetName()) +func closeFiles(endpoint *registry.NetworkServiceEndpoint, fileMaps *genericsync.Map[string, *perEndpointFileMap]) { + defer fileMaps.Delete(endpoint.GetName()) - fileMap, _ := r.fileMaps.LoadOrStore(endpoint.GetName(), &perEndpointFileMap{ - filesByInodeURL: make(map[string]*os.File), - inodeURLbyFilename: make(map[string]*url.URL), - }) + fileMap, loaded := fileMaps.LoadAndDelete(endpoint.GetName()) + if !loaded { + return + } for inodeURLStr, file := range fileMap.filesByInodeURL { delete(fileMap.filesByInodeURL, inodeURLStr) _ = file.Close() + _ = os.Remove(file.Name()) } }