Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@

# Dependency directories (remove the comment below to include it)
# vendor/
.vscode/
76 changes: 76 additions & 0 deletions extensions/lock/lock.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package lock

import (
"errors"
"io/fs"
"os"
"sync"
"time"

"github.com/AzureAD/microsoft-authentication-extensions-for-go/flock"
)

type Lock struct {
retries int
retryDelay time.Duration

fLock *flock.Flock
mu sync.Mutex
}

type Option func(l *Lock)

func WithRetries(n int) Option {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Public requires a comment. I'd put:

// WithRetries changes the default number of retries from [retries] to n. Negative values panic()

Also, add panic on negative values.

return func(l *Lock) {
l.retries = n
}
}
func WithRetryDelay(t time.Duration) Option {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Public requires a comment. I'd put:

// WithRetryDelay changes the default delay from [delay] to t.

return func(l *Lock) {
l.retryDelay = t
}
}

func New(lockFileName string, options ...Option) (*Lock, error) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lockFileName can be just "p".

// New creates a Lock for a lock file at "p". If "p" doesn't exist, it will be created when using. Lock().
func New(p string options ...Option) (*Lock, error)

l := &Lock{}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to add default value for retries and retryDelay.

for _, o := range options {
o(l)
}
l.fLock = flock.New(lockFileName)
return l, nil
}

func (l *Lock) Lock() error {
l.mu.Lock()
defer l.mu.Unlock()
for tryCount := 0; tryCount < l.retries; tryCount++ {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for i := 0; i < l.retries; i++ {
...

locked, err := l.fLock.TryLock()
if err != nil || !locked {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If locked {
return nil
}
time.Sleep(l.retryDelay * time.Millisecond)

time.Sleep(l.retryDelay * time.Millisecond)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be:
time.Sleep(l.retryDelay)

continue
} else if locked {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove "else if locked" section

// commenting this till we have flock merged in and ready to use with changes
// otherwise it would not compile
//l.fLock.Fh.WriteString(fmt.Sprintf("{%d} {%s}", os.Getpid(), os.Args[0]))
//l.fLock.Fh.Sync()
return nil
}
}
return errors.New("failed to acquire lock")
}

func (l *Lock) UnLock() error {
if l.fLock != nil {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the "if l.fLock != nil", doesn't provide value.

if err := l.fLock.Unlock(); err != nil {
return err
}
err := os.Remove(l.fLock.Path())
if err != nil {
errVal := err.(*fs.PathError)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if errors.Is(err, fs. ErrNotExist) || errors.Is(err, fs. ErrPermission) {
return nil
}
return err

if errVal != fs.ErrNotExist || errVal != fs.ErrPermission {
return err
}
}
}
return nil
}
121 changes: 121 additions & 0 deletions extensions/lock/lock_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
package lock

import (
"bytes"
"errors"
"fmt"
"io/ioutil"
"log"
"os"
"strings"
"sync"
"testing"
"time"

"github.com/AzureAD/microsoft-authentication-extensions-for-go/internal"
)

const cacheFile = "cache.txt"

func TestLocking(t *testing.T) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove blank line

tests := []struct {
desc string
concurrency int
sleepInterval time.Duration
cacheFile string
}{
{"Normal", 4, 50 * time.Millisecond, "cache_normal"},
{"High", 40, 50 * time.Millisecond, "cache_high"},
}

for _, test := range tests {
tmpfile, err := ioutil.TempFile("", test.cacheFile)
defer os.Remove(tmpfile.Name())
if err != nil {
t.Fatalf("TestLocking(%s): Could not create cache file", test.desc)
}
err = spin(test.concurrency, time.Duration(test.sleepInterval), tmpfile.Name())
if err != nil {
t.Fatalf("TestLocking(%s): %s", test.desc, err)
}
}

}
func acquire(threadNo int, sleepInterval time.Duration, cacheFile string) {
cacheAccessor := internal.NewFileAccessor(cacheFile)
lockfileName := cacheFile + ".lockfile"
l, err := New(lockfileName, WithRetries(60), WithRetryDelay(100))
if err := l.Lock(); err != nil {
log.Println("Couldn't acquire lock", err.Error())
return
}
defer l.UnLock()
data, err := cacheAccessor.Read()
if err != nil {
log.Println(err)
}
var buffer bytes.Buffer
buffer.Write(data)
buffer.WriteString(fmt.Sprintf("< %d \n", threadNo))
time.Sleep(sleepInterval)
buffer.WriteString(fmt.Sprintf("> %d \n", threadNo))
cacheAccessor.Write(buffer.Bytes())
}

func spin(concurrency int, sleepInterval time.Duration, cacheFile string) error {
var wg sync.WaitGroup
wg.Add(concurrency)
for i := 0; i < concurrency; i++ {
go func(i int) {
defer wg.Done()
acquire(i, sleepInterval, cacheFile)
}(i)
}
wg.Wait()
i, err := validate(cacheFile)
if err != nil {
return err
}
if i != concurrency*2 {
return fmt.Errorf("should have seen %d line entries, found %d", concurrency*2, i)
}
return nil
}

func validate(cacheFile string) (int, error) {
var (
count int
prevProc, tag, proc string
)
data, err := os.ReadFile(cacheFile)
if err != nil {
log.Println(err)
}
temp := strings.Split(string(data), "\n")

for _, s := range temp {
if strings.TrimSpace(s) == "" {
continue
}
count += 1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

count++

split := strings.Split(s, " ")
tag = split[0]
proc = split[1]
if prevProc == "" {
if tag != "<" {
return 0, errors.New("opening bracket not found")
}
prevProc = proc
continue
}
if proc != prevProc {
return 0, errors.New("process overlap found")
}
if tag != ">" {
return 0, errors.New("process overlap found")
}
prevProc = ""
}
return count, nil
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add two tests:

One tests all the conditions of Lock().
Another tests all the conditions of Unlock()

  • flock.Unlock() has error
  • Lockfile can't be removed

You can do this by providing an interface:

type flocker interface{
TryLock() error
Unlock() error
Path() string
}

type fakeFlock struct {
err bool
}

func (f fakeFlock) TryLock() error {
if f.err {
return errors.New("error")
}
return nil
}

func (f fakeFlock) Unlock() error {
if f.err {
return errors.New("error")
}
return nil
}

10 changes: 10 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module github.com/AzureAD/microsoft-authentication-extensions-for-go

go 1.14

require (
github.com/AzureAD/microsoft-authentication-library-for-go v0.3.0
github.com/billgraziano/dpapi v0.4.0
github.com/gofrs/flock v0.8.1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This flock line shouldn't. be here. So either a path change is needed or you need to run go tidy


)
Loading