-
Notifications
You must be signed in to change notification settings - Fork 1
Cross Platform lock + windows encryption #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 13 commits
180b593
187858e
93a1abd
43149eb
cbd3859
b231fb4
54f1c22
15852ec
f973d88
e32eddf
d139ff0
c825fca
9c21596
666b417
357fd53
53249aa
92fe187
d4c10f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,3 +13,4 @@ | |
|
|
||
| # Dependency directories (remove the comment below to include it) | ||
| # vendor/ | ||
| .vscode/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| package lock | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "os" | ||
| "sync" | ||
| "time" | ||
|
|
||
| "github.com/AzureAD/microsoft-authentication-extensions-for-go/flock" | ||
| ) | ||
|
|
||
| type Lock struct { | ||
| retries int | ||
| retryDelay time.Duration | ||
|
|
||
| lockFile *os.File | ||
| lockfileName string | ||
|
|
||
| fLock *flock.Flock | ||
| mu sync.Mutex | ||
| } | ||
|
|
||
| type Option func(l *Lock) | ||
|
|
||
| func WithRetries(n int) Option { | ||
| return func(l *Lock) { | ||
| l.retries = n | ||
| } | ||
| } | ||
| func WithRetryDelay(t time.Duration) Option { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(). |
||
| l := &Lock{} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
| l.lockfileName = lockFileName | ||
| return l, nil | ||
| } | ||
|
|
||
| func (l *Lock) Lock() error { | ||
| l.mu.Lock() | ||
| defer l.mu.Unlock() | ||
| for tryCount := 0; tryCount < l.retries; tryCount++ { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for i := 0; i < l.retries; i++ { |
||
| lockfile, err := os.OpenFile(lockFileName, os.O_RDWR|os.O_CREATE) | ||
| if err != nil { | ||
| return &Lock{}, err | ||
| } | ||
| err := l.fLock.Lock() | ||
| if err != nil { | ||
| time.Sleep(l.retryDelay * time.Millisecond) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be: |
||
| continue | ||
| } else { | ||
| if l.fLock.Locked() { | ||
| l.fLock.Fh.WriteString(fmt.Sprintf("{%d} {%s}", os.Getpid(), os.Args[0])) | ||
| } | ||
| return nil | ||
| } | ||
| } | ||
| return errors.New("failed to acquire lock") | ||
| } | ||
|
|
||
| func (l *Lock) UnLock() error { | ||
| if l.fLock != nil { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
| } | ||
| l.lockFile.Close() | ||
| if err := os.Remove(l.fLock.Path()); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| package lock | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "fmt" | ||
| "log" | ||
| "os" | ||
| "strings" | ||
| "sync" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/AzureAD/microsoft-authentication-extensions-for-go/internal" | ||
| ) | ||
|
|
||
| func spinThreads(noOfThreads int, sleepInterval time.Duration, t *testing.T) int { | ||
| cacheFile := "cache.txt" | ||
| var wg sync.WaitGroup | ||
| wg.Add(noOfThreads) | ||
| for i := 0; i < noOfThreads; i++ { | ||
| go func(i int) { | ||
| defer wg.Done() | ||
| acquireLockAndWriteToCache(i, sleepInterval, cacheFile) | ||
| }(i) | ||
| } | ||
| wg.Wait() | ||
| t.Cleanup(func() { | ||
| if err := os.Remove(cacheFile); err != nil { | ||
| log.Println("Failed to remove cache file", err) | ||
| } | ||
| }) | ||
| return validateResult(cacheFile, t) | ||
| } | ||
|
|
||
| func acquireLockAndWriteToCache(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 * time.Millisecond) | ||
| buffer.WriteString(fmt.Sprintf("> %d \n", threadNo)) | ||
| cacheAccessor.Write(buffer.Bytes()) | ||
| } | ||
|
|
||
| func validateResult(cacheFile string, t *testing.T) int { | ||
| count := 0 | ||
| var prevProc string = "" | ||
|
||
| var tag string | ||
| var proc string | ||
| data, err := os.ReadFile(cacheFile) | ||
| if err != nil { | ||
| log.Println(err) | ||
| } | ||
| dat := string(data) | ||
|
||
| temp := strings.Split(dat, "\n") | ||
| for _, ele := range temp { | ||
| if ele != "" { | ||
| count += 1 | ||
| split := strings.Split(ele, " ") | ||
| tag = split[0] | ||
| proc = split[1] | ||
| if prevProc != "" { | ||
| if proc != prevProc { | ||
| t.Fatal("Process overlap found") | ||
| } | ||
| if tag != ">" { | ||
| t.Fatal("Process overlap found") | ||
| } | ||
| prevProc = "" | ||
|
|
||
| } else { | ||
| if tag != "<" { | ||
| t.Fatal("Opening bracket not found") | ||
| } | ||
| prevProc = proc | ||
| } | ||
| } | ||
| } | ||
| return count | ||
| } | ||
| func TestForNormalWorkload(t *testing.T) { | ||
| noOfThreads := 4 | ||
| sleepInterval := 100 | ||
| n := spinThreads(noOfThreads, time.Duration(sleepInterval), t) | ||
| if n != 4*2 { | ||
| t.Fatalf("Should not observe starvation") | ||
| } | ||
| } | ||
|
|
||
| func TestForHighWorkload(t *testing.T) { | ||
| noOfThreads := 80 | ||
| sleepInterval := 100 | ||
| n := spinThreads(noOfThreads, time.Duration(sleepInterval), t) | ||
| if n > 80*2 { | ||
| t.Fatalf("Starvation or not, we should not observe garbled payload") | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add two tests: One tests all the conditions of Lock().
You can do this by providing an interface: type flocker interface{ type fakeFlock struct { func (f fakeFlock) TryLock() error { func (f fakeFlock) Unlock() error { |
||
| 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 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
|
||
| ) | ||
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| package internal | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move these files under: internal/accessor accessor.go:
All "Accessor" types implement accessor.Cache |
||
|
|
||
| import ( | ||
| "io/ioutil" | ||
| "os" | ||
| ) | ||
|
|
||
| type cacheAccessor interface { | ||
| Read() ([]byte, error) | ||
| Write(data []byte) | ||
| Delete() | ||
| } | ||
|
|
||
| type FileAccessor struct { | ||
| cacheFilePath string | ||
| } | ||
|
|
||
| func NewFileAccessor(cacheFilePath string) *FileAccessor { | ||
| return &FileAccessor{cacheFilePath: cacheFilePath} | ||
| } | ||
|
|
||
| func (f *FileAccessor) Read() ([]byte, error) { | ||
| var data []byte | ||
| file, err := os.Open(f.cacheFilePath) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer file.Close() | ||
| data, err = ioutil.ReadAll(file) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return data, nil | ||
| } | ||
|
|
||
| func (f *FileAccessor) Write(data []byte) error { | ||
| err := ioutil.WriteFile(f.cacheFilePath, data, 0600) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func (f *FileAccessor) WriteAtomic(data []byte) { | ||
| // Not implemented yet | ||
| } | ||
|
|
||
| func (f *FileAccessor) Delete() { | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| //go:build windows | ||
| // +build windows | ||
|
|
||
| package internal | ||
|
|
||
| import ( | ||
| "io/ioutil" | ||
| "os" | ||
| "runtime" | ||
|
|
||
| "github.com/billgraziano/dpapi" | ||
| ) | ||
|
|
||
| type WindowsAccessor struct { | ||
| cacheFilePath string | ||
| } | ||
|
|
||
| func NewWindowsAccessor(cacheFilePath string) *WindowsAccessor { | ||
| return &WindowsAccessor{cacheFilePath: cacheFilePath} | ||
| } | ||
|
|
||
| func (w *WindowsAccessor) Read() ([]byte, error) { | ||
| var data []byte | ||
| file, err := os.Open(w.cacheFilePath) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer file.Close() | ||
| data, err = ioutil.ReadAll(file) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if data != nil && len(data) != 0 && runtime.GOOS == "windows" { | ||
| data, err = dpapi.DecryptBytes(data) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
| return data, nil | ||
| } | ||
|
|
||
| func (w *WindowsAccessor) Write(data []byte) error { | ||
| data, err := dpapi.EncryptBytes(data) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| err = ioutil.WriteFile(w.cacheFilePath, data, 0600) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func (w *WindowsAccessor) WriteAtomic(data []byte) { | ||
| // Not implemented yet | ||
| } | ||
|
|
||
| func (w *WindowsAccessor) Delete() { | ||
|
|
||
| } |
There was a problem hiding this comment.
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.