diff --git a/go.mod b/go.mod index bff90a819..9990f842e 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/AlecAivazis/survey/v2 v2.3.6 github.com/Masterminds/semver v1.5.0 github.com/adrg/xdg v0.4.0 + github.com/alexflint/go-filemutex v1.3.0 github.com/cppforlife/go-cli-ui v0.0.0-20220425131040-94f26b16bc14 github.com/fatih/color v1.15.0 github.com/gobwas/glob v0.2.3 @@ -19,7 +20,6 @@ require ( github.com/google/uuid v1.6.0 github.com/gorilla/mux v1.8.0 github.com/imdario/mergo v0.3.13 - github.com/juju/fslock v0.0.0-20160525022230-4d5c94c67b4b github.com/k14s/kbld v0.32.0 github.com/lithammer/dedent v1.1.0 github.com/logrusorgru/aurora v2.0.3+incompatible @@ -155,6 +155,7 @@ require ( github.com/jmespath/go-jmespath v0.4.0 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect + github.com/juju/fslock v0.0.0-20160525022230-4d5c94c67b4b // indirect github.com/k14s/semver/v4 v4.0.1-0.20210701191048-266d47ac6115 // indirect github.com/k14s/starlark-go v0.0.0-20200720175618-3a5c849cc368 // indirect github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect diff --git a/go.sum b/go.sum index e4ef981b6..9a3d6f102 100644 --- a/go.sum +++ b/go.sum @@ -70,6 +70,8 @@ github.com/VividCortex/ewma v1.1.1 h1:MnEK4VOv6n0RSY4vtRe3h11qjxL3+t0B8yOL8iMXdc github.com/VividCortex/ewma v1.1.1/go.mod h1:2Tkkvm3sRDVXaiyucHiACn4cqf7DpdyLvmxzcbUokwA= github.com/adrg/xdg v0.4.0 h1:RzRqFcjH4nE5C6oTAxhBtoE2IRyjBSa62SCbyPidvls= github.com/adrg/xdg v0.4.0/go.mod h1:N6ag73EX4wyxeaoeHctc1mas01KZgsj5tYiAIwqJE/E= +github.com/alexflint/go-filemutex v1.3.0 h1:LgE+nTUWnQCyRKbpoceKZsPQbs84LivvgwUymZXdOcM= +github.com/alexflint/go-filemutex v1.3.0/go.mod h1:U0+VA/i30mGBlLCrFPGtTe9y6wGQfNAWPBTekHQ+c8A= github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= github.com/asaskevich/govalidator v0.0.0-20200907205600-7a23bdc65eef/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so= @@ -940,6 +942,7 @@ golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220906165534-d0df966e6959/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= diff --git a/pkg/telemetry/metrics_db_lock.go b/pkg/telemetry/metrics_db_lock.go index 99372893f..71ba40ab3 100644 --- a/pkg/telemetry/metrics_db_lock.go +++ b/pkg/telemetry/metrics_db_lock.go @@ -10,11 +10,9 @@ import ( "sync" "time" - "github.com/pkg/errors" + "github.com/alexflint/go-filemutex" "github.com/vmware-tanzu/tanzu-cli/pkg/common" - - "github.com/juju/fslock" ) const ( @@ -27,7 +25,7 @@ var cliMetricDBLockFile string // cliMetricDBLock used as a static lock variable that stores fslock // This is used for interprocess locking of the tanzu cli metrics DB file -var cliMetricDBLock *fslock.Lock +var cliMetricDBLock *filemutex.FileMutex // cliMetricDBMutex is used to handle the locking behavior between concurrent calls // within the existing process trying to acquire the lock @@ -68,7 +66,7 @@ func ReleaseTanzuMetricDBLock() { } // getFileLockWithTimeout returns a file lock with timeout -func getFileLockWithTimeout(lockPath string, lockDuration time.Duration) (*fslock.Lock, error) { +func getFileLockWithTimeout(lockPath string, lockDuration time.Duration) (*filemutex.FileMutex, error) { dir := filepath.Dir(lockPath) if _, err := os.Stat(dir); os.IsNotExist(err) { @@ -77,10 +75,28 @@ func getFileLockWithTimeout(lockPath string, lockDuration time.Duration) (*fsloc } } - lock := fslock.New(lockPath) + flock, err := filemutex.New(lockPath) + if err != nil { + return nil, err + } - if err := lock.LockWithTimeout(lockDuration); err != nil { - return nil, errors.Wrap(err, "failed to acquire a lock with timeout") + result := make(chan error) + cancel := make(chan struct{}) + go func() { + err := flock.Lock() + select { + case <-cancel: + // Timed out, cleanup if necessary. + _ = flock.Unlock() + case result <- err: + } + }() + + select { + case err := <-result: + return flock, err + case <-time.After(lockDuration): + close(cancel) + return flock, fmt.Errorf("timeout waiting for lock") } - return lock, nil } diff --git a/pkg/telemetry/metrics_db_lock_test.go b/pkg/telemetry/metrics_db_lock_test.go new file mode 100644 index 000000000..a0f8ef998 --- /dev/null +++ b/pkg/telemetry/metrics_db_lock_test.go @@ -0,0 +1,107 @@ +// Copyright 2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package telemetry + +import ( + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestAcquireAndReleaseTanzuMetricDBLock(t *testing.T) { + // Test acquiring the lock + err := AcquireTanzuMetricDBLock() + assert.NoError(t, err, "Expected no error while acquiring the lock") + + // Verify the lock is held + assert.NotNil(t, cliMetricDBLock, "Expected the lock to be acquired") + + // Test releasing the lock + assert.NotPanics(t, func() { + ReleaseTanzuMetricDBLock() + }, "Expected no panic while releasing the lock") + + // Verify the lock is released + assert.Nil(t, cliMetricDBLock, "Expected the lock to be released") +} + +func TestLockTimeout(t *testing.T) { + // Acquire the lock for the first time + err := AcquireTanzuMetricDBLock() + assert.NoError(t, err, "Expected no error while acquiring the lock the first time") + + // Try acquiring the lock again, should time out + err = AcquireTanzuMetricDBLock() + assert.Error(t, err, "Expected a timeout error while trying to acquire the lock again") + assert.ErrorContains(t, err, "timeout waiting for lock") + + // Release the initial lock + ReleaseTanzuMetricDBLock() +} + +func TestMultipleAcquireAndRelease(t *testing.T) { + // Acquire and release the lock multiple times + for i := 0; i < 3; i++ { + err := AcquireTanzuMetricDBLock() + assert.NoError(t, err, "Expected no error while acquiring the lock") + + assert.NotNil(t, cliMetricDBLock, "Expected the lock to be acquired") + + assert.NotPanics(t, func() { + ReleaseTanzuMetricDBLock() + }, "Expected no panic while releasing the lock") + + assert.Nil(t, cliMetricDBLock, "Expected the lock to be released") + } +} + +func TestParallelLocking(t *testing.T) { + const goroutines = 10 + var wg sync.WaitGroup + wg.Add(goroutines) + successCount := int32(0) + + for i := 0; i < goroutines; i++ { + go func() { + defer wg.Done() + err := AcquireTanzuMetricDBLock() + if err == nil { + // sleep and hold lock for more than timeout period + // so that all other go routines fail to acquire lock + time.Sleep(2 * DefaultMetricsDBLockTimeout) + defer ReleaseTanzuMetricDBLock() + successCount++ + } + }() + } + + wg.Wait() + assert.Equal(t, int32(1), successCount, "Expected only one goroutine to successfully acquire the lock") +} + +func TestParallelLockingAndUnlocking(t *testing.T) { + const goroutines = 10 + var wg sync.WaitGroup + wg.Add(goroutines) + successCount := int32(0) + + for i := 0; i < goroutines; i++ { + go func() { + defer wg.Done() + err := AcquireTanzuMetricDBLock() + if err == nil { + // sleep and hold lock for less than timeout period + // so that all other go routines could acquire and release lock successfully + time.Sleep(100 * time.Millisecond) + defer ReleaseTanzuMetricDBLock() + successCount++ + } + }() + } + + wg.Wait() + assert.Equal(t, int32(10), successCount, "Expected all the goroutine to successfully acquire the lock") +}