Fix fd mem leaks in nsmgr and forwarder

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
 	}
 

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,19 +61,22 @@ 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
 		}
 	}
 
 	// 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())
 	}
 }

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
 }

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())
 	}
 }