From a5c29a3847b66f70cdc2d9a6dc3301492b00a6ee Mon Sep 17 00:00:00 2001 From: Blaine Gardner Date: Thu, 10 Jun 2021 10:50:30 -0600 Subject: [PATCH] ceph: handle CephCluster stopCleanupCh better Investigate a possible memory leak around CephCluster cleanup's stopCleanupCh channel. Add more close() statements and comment that Go's garbage collector is able to close the channel when there are no more referents to the channel. Signed-off-by: Blaine Gardner --- pkg/operator/ceph/cluster/controller.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/operator/ceph/cluster/controller.go b/pkg/operator/ceph/cluster/controller.go index 9832a78ff..5680e5d29 100644 --- a/pkg/operator/ceph/cluster/controller.go +++ b/pkg/operator/ceph/cluster/controller.go @@ -340,9 +340,15 @@ func (r *ReconcileCephCluster) reconcileDelete(cephCluster *cephv1.CephCluster) if doCleanup { cephHosts, err := r.clusterController.getCephHosts(cephCluster.Namespace) if err != nil { + close(stopCleanupCh) return reconcile.Result{}, cephCluster, errors.Wrapf(err, "failed to find valid ceph hosts in the cluster %q", cephCluster.Namespace) } + // Go will garbage collect the stopCleanupCh if it is left open once the cluster cleanup + // goroutine is no longer running (i.e., referencing the channel) go r.clusterController.startClusterCleanUp(stopCleanupCh, cephCluster, cephHosts, monSecret, clusterFSID) + } else { + // stop channel not needed if the cleanup goroutine isn't started + close(stopCleanupCh) } } @@ -352,7 +358,6 @@ func (r *ReconcileCephCluster) reconcileDelete(cephCluster *cephv1.CephCluster) if err != nil { // If the cluster cannot be deleted, requeue the request for deletion to see if the conditions // will eventually be satisfied such as the volumes being removed - // TODO: is it a memory leak to not close the channel in other cases? close(stopCleanupCh) return response, cephCluster, errors.Wrapf(err, "failed to clean up CephCluster %q", nsName.String()) }