From f6ca94ac18a67815562170eba31bc4b7b9b663cd Mon Sep 17 00:00:00 2001
From: Vlad <13818348+walldiss@users.noreply.github.com>
Date: Thu, 8 Aug 2024 18:00:42 +0200
Subject: [PATCH 1/3] force close Accessor if not closed within timeout close
 Accessor and log Error if ref was removed by GC instead of user

---
 share/eds/cache/accessor_cache.go | 36 +++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/share/eds/cache/accessor_cache.go b/share/eds/cache/accessor_cache.go
index a45c2542f8..6781d14506 100644
--- a/share/eds/cache/accessor_cache.go
+++ b/share/eds/cache/accessor_cache.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"runtime"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -97,10 +98,12 @@ func (s *accessorWithBlockstore) close() error {
 	done := s.done
 	s.Unlock()
 
+	// wait until all references are released or timeout is reached. If timeout is reached, log an
+	// error and close the accessor forcefully.
 	select {
 	case <-done:
 	case <-time.After(defaultCloseTimeout):
-		return fmt.Errorf("closing accessor, some readers didn't close the accessor within timeout,"+
+		log.Errorf("closing accessor, some readers didn't close the accessor within timeout,"+
 			" amount left: %v", s.refs.Load())
 	}
 	if err := s.shardAccessor.Close(); err != nil {
@@ -235,7 +238,8 @@ func (bc *AccessorCache) EnableMetrics() (CloseMetricsFn, error) {
 // Close is called
 type refCloser struct {
 	*accessorWithBlockstore
-	closeFn func()
+	closed    atomic.Bool
+	removeRef func()
 }
 
 // newRefCloser creates new refCloser
@@ -244,17 +248,31 @@ func newRefCloser(abs *accessorWithBlockstore) (*refCloser, error) {
 		return nil, err
 	}
 
-	var closeOnce sync.Once
-	return &refCloser{
+	rf := &refCloser{
 		accessorWithBlockstore: abs,
-		closeFn: func() {
-			closeOnce.Do(abs.removeRef)
-		},
-	}, nil
+		removeRef:              abs.removeRef,
+	}
+	// Set finalizer to ensure that accessor is closed when refCloser is garbage collected.
+	// We expect that refCloser will be closed explicitly by the caller. If it is not closed,
+	// we log an error.
+	runtime.SetFinalizer(rf, func(rf *refCloser) {
+		if rf.close() {
+			log.Errorf("refCloser for accessor was garbage collected before Close was called")
+		}
+	})
+	return rf, nil
+}
+
+func (c *refCloser) close() bool {
+	if c.closed.CompareAndSwap(false, true) {
+		c.removeRef()
+		return true
+	}
+	return false
 }
 
 func (c *refCloser) Close() error {
-	c.closeFn()
+	c.close()
 	return nil
 }
 

From 553aad2c47074e3c06d8b82ef061a5c3e5536ef8 Mon Sep 17 00:00:00 2001
From: Vlad <13818348+walldiss@users.noreply.github.com>
Date: Wed, 14 Aug 2024 14:40:49 +0200
Subject: [PATCH 2/3] Update share/eds/cache/accessor_cache.go

Co-authored-by: rene <41963722+renaynay@users.noreply.github.com>
---
 share/eds/cache/accessor_cache.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/share/eds/cache/accessor_cache.go b/share/eds/cache/accessor_cache.go
index 6781d14506..3b9fc90024 100644
--- a/share/eds/cache/accessor_cache.go
+++ b/share/eds/cache/accessor_cache.go
@@ -257,7 +257,7 @@ func newRefCloser(abs *accessorWithBlockstore) (*refCloser, error) {
 	// we log an error.
 	runtime.SetFinalizer(rf, func(rf *refCloser) {
 		if rf.close() {
-			log.Errorf("refCloser for accessor was garbage collected before Close was called")
+			log.Error("refCloser for accessor was garbage collected before Close was called")
 		}
 	})
 	return rf, nil

From 4598bc977e8ca8b9133b12d78525fbe7ab717470 Mon Sep 17 00:00:00 2001
From: Vlad <13818348+walldiss@users.noreply.github.com>
Date: Mon, 19 Aug 2024 15:04:55 +0200
Subject: [PATCH 3/3] remove finilisier

---
 share/eds/cache/accessor_cache.go | 32 ++++++++-----------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/share/eds/cache/accessor_cache.go b/share/eds/cache/accessor_cache.go
index 3b9fc90024..e7ac043426 100644
--- a/share/eds/cache/accessor_cache.go
+++ b/share/eds/cache/accessor_cache.go
@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"runtime"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -238,8 +237,7 @@ func (bc *AccessorCache) EnableMetrics() (CloseMetricsFn, error) {
 // Close is called
 type refCloser struct {
 	*accessorWithBlockstore
-	closed    atomic.Bool
-	removeRef func()
+	closeFn func()
 }
 
 // newRefCloser creates new refCloser
@@ -248,31 +246,17 @@ func newRefCloser(abs *accessorWithBlockstore) (*refCloser, error) {
 		return nil, err
 	}
 
-	rf := &refCloser{
+	var closeOnce sync.Once
+	return &refCloser{
 		accessorWithBlockstore: abs,
-		removeRef:              abs.removeRef,
-	}
-	// Set finalizer to ensure that accessor is closed when refCloser is garbage collected.
-	// We expect that refCloser will be closed explicitly by the caller. If it is not closed,
-	// we log an error.
-	runtime.SetFinalizer(rf, func(rf *refCloser) {
-		if rf.close() {
-			log.Error("refCloser for accessor was garbage collected before Close was called")
-		}
-	})
-	return rf, nil
-}
-
-func (c *refCloser) close() bool {
-	if c.closed.CompareAndSwap(false, true) {
-		c.removeRef()
-		return true
-	}
-	return false
+		closeFn: func() {
+			closeOnce.Do(abs.removeRef)
+		},
+	}, nil
 }
 
 func (c *refCloser) Close() error {
-	c.close()
+	c.closeFn()
 	return nil
 }