[lxc-devel] [lxd/master] Fix race in cancel code and make golint clean
stgraber on Github
lxc-bot at linuxcontainers.org
Fri Feb 16 19:44:05 UTC 2018
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20180216/3112c6bd/attachment.bin>
-------------- next part --------------
From a32f083d562f4981204d7c8a624281390cfec90e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Fri, 16 Feb 2018 14:39:52 -0500
Subject: [PATCH 1/2] shared/cancel: Properly lock map
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
shared/cancel/canceler.go | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/shared/cancel/canceler.go b/shared/cancel/canceler.go
index 6a27347a0..44059981d 100644
--- a/shared/cancel/canceler.go
+++ b/shared/cancel/canceler.go
@@ -3,21 +3,31 @@ package cancel
import (
"fmt"
"net/http"
+ "sync"
)
// A struct to track canceleation
type Canceler struct {
reqChCancel map[*http.Request]chan struct{}
+ lock sync.Mutex
}
func NewCanceler() *Canceler {
c := Canceler{}
+
+ c.lock.Lock()
c.reqChCancel = make(map[*http.Request]chan struct{})
+ c.lock.Unlock()
+
return &c
}
func (c *Canceler) Cancelable() bool {
- return len(c.reqChCancel) > 0
+ c.lock.Lock()
+ length := len(c.reqChCancel)
+ c.lock.Unlock()
+
+ return length > 0
}
func (c *Canceler) Cancel() error {
@@ -25,10 +35,13 @@ func (c *Canceler) Cancel() error {
return fmt.Errorf("This operation cannot be canceled at this time")
}
+ c.lock.Lock()
for req, ch := range c.reqChCancel {
close(ch)
delete(c.reqChCancel, req)
}
+ c.lock.Unlock()
+
return nil
}
@@ -36,14 +49,18 @@ func CancelableDownload(c *Canceler, client *http.Client, req *http.Request) (*h
chDone := make(chan bool)
chCancel := make(chan struct{})
if c != nil {
+ c.lock.Lock()
c.reqChCancel[req] = chCancel
+ c.lock.Unlock()
}
req.Cancel = chCancel
go func() {
<-chDone
if c != nil {
+ c.lock.Lock()
delete(c.reqChCancel, req)
+ c.lock.Unlock()
}
}()
From c4ef9a68cb62dbc70cacf440708bd9096197ce5e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Fri, 16 Feb 2018 14:43:30 -0500
Subject: [PATCH 2/2] shared/cancel: Make the cancel code golint clean
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
shared/cancel/canceler.go | 8 ++++++--
test/suites/static_analysis.sh | 1 +
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/shared/cancel/canceler.go b/shared/cancel/canceler.go
index 44059981d..b3356cf37 100644
--- a/shared/cancel/canceler.go
+++ b/shared/cancel/canceler.go
@@ -6,12 +6,13 @@ import (
"sync"
)
-// A struct to track canceleation
+// Canceler tracks a cancelable operation
type Canceler struct {
reqChCancel map[*http.Request]chan struct{}
lock sync.Mutex
}
+// NewCanceler returns a new Canceler struct
func NewCanceler() *Canceler {
c := Canceler{}
@@ -22,6 +23,7 @@ func NewCanceler() *Canceler {
return &c
}
+// Cancelable indicates whether there are operations that support cancelation
func (c *Canceler) Cancelable() bool {
c.lock.Lock()
length := len(c.reqChCancel)
@@ -30,9 +32,10 @@ func (c *Canceler) Cancelable() bool {
return length > 0
}
+// Cancel will attempt to cancel all ongoing operations
func (c *Canceler) Cancel() error {
if !c.Cancelable() {
- return fmt.Errorf("This operation cannot be canceled at this time")
+ return fmt.Errorf("This operation can't be canceled at this time")
}
c.lock.Lock()
@@ -45,6 +48,7 @@ func (c *Canceler) Cancel() error {
return nil
}
+// CancelableDownload performs an http request and allows for it to be canceled at any time
func CancelableDownload(c *Canceler, client *http.Client, req *http.Request) (*http.Response, chan bool, error) {
chDone := make(chan bool)
chCancel := make(chan struct{})
diff --git a/test/suites/static_analysis.sh b/test/suites/static_analysis.sh
index 81b8dd67f..425287a42 100644
--- a/test/suites/static_analysis.sh
+++ b/test/suites/static_analysis.sh
@@ -58,6 +58,7 @@ test_static_analysis() {
golint -set_exit_status client/
golint -set_exit_status lxc/config/
golint -set_exit_status shared/api/
+ golint -set_exit_status shared/cancel/
golint -set_exit_status shared/cmd/
golint -set_exit_status shared/gnuflag/
golint -set_exit_status shared/i18n/
More information about the lxc-devel
mailing list