Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Revert "fix memory leaks (#1616)"
Browse files Browse the repository at this point in the history
This reverts commit 1cbc041.
NikitaSkrynnik committed Jun 17, 2024
1 parent e3eed82 commit a763a15
Showing 4 changed files with 16 additions and 33 deletions.
3 changes: 0 additions & 3 deletions pkg/networkservice/common/mechanisms/recvfd/client.go
Original file line number Diff line number Diff line change
@@ -2,8 +2,6 @@
//
// 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,7 +68,6 @@ 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
}

21 changes: 9 additions & 12 deletions pkg/networkservice/common/mechanisms/recvfd/server.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// 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");
@@ -61,22 +59,19 @@ 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
}

@@ -85,7 +80,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 closeFiles(conn, &r.fileMaps)
defer r.closeFiles(conn)

// Get the grpcfd.FDRecver
recv, ok := grpcfd.FromContext(ctx)
@@ -116,14 +111,16 @@ func (r *recvFDServer) Close(ctx context.Context, conn *networkservice.Connectio
return &empty.Empty{}, err
}

func closeFiles(conn *networkservice.Connection, fileMaps *genericsync.Map[string, *perConnectionFileMap]) {
fileMap, loaded := fileMaps.LoadAndDelete(conn.GetId())
if !loaded {
return
}
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),
})

for inodeURLStr, file := range fileMap.filesByInodeURL {
delete(fileMap.filesByInodeURL, inodeURLStr)
_ = file.Close()
_ = os.Remove(file.Name())
}
}
5 changes: 0 additions & 5 deletions pkg/registry/common/recvfd/client.go
Original file line number Diff line number Diff line change
@@ -2,8 +2,6 @@
//
// 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");
@@ -101,9 +99,6 @@ 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
}
20 changes: 7 additions & 13 deletions pkg/registry/common/recvfd/server.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// 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");
@@ -72,14 +70,12 @@ 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()
@@ -91,7 +87,6 @@ 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
@@ -107,7 +102,7 @@ func (r *recvfdNseServer) Unregister(ctx context.Context, endpoint *registry.Net
}

// Clean up the fileMap no matter what happens
defer closeFiles(endpoint, &r.fileMaps)
defer r.closeFiles(endpoint)

// Get the grpcfd.FDRecver
recv, ok := grpcfd.FromContext(ctx)
@@ -219,17 +214,16 @@ func swapFileToInode(fileMap *perEndpointFileMap, endpoint *registry.NetworkServ
return nil
}

func closeFiles(endpoint *registry.NetworkServiceEndpoint, fileMaps *genericsync.Map[string, *perEndpointFileMap]) {
defer fileMaps.Delete(endpoint.GetName())
func (r *recvfdNseServer) closeFiles(endpoint *registry.NetworkServiceEndpoint) {
defer r.fileMaps.Delete(endpoint.GetName())

fileMap, loaded := fileMaps.LoadAndDelete(endpoint.GetName())
if !loaded {
return
}
fileMap, _ := r.fileMaps.LoadOrStore(endpoint.GetName(), &perEndpointFileMap{
filesByInodeURL: make(map[string]*os.File),
inodeURLbyFilename: make(map[string]*url.URL),
})

for inodeURLStr, file := range fileMap.filesByInodeURL {
delete(fileMap.filesByInodeURL, inodeURLStr)
_ = file.Close()
_ = os.Remove(file.Name())
}
}

0 comments on commit a763a15

Please sign in to comment.