[lxc-devel] [lxd/master] Operations: Fixes race condition and unsafe Operations() function
tomponline on Github
lxc-bot at linuxcontainers.org
Mon Jul 20 13:42:23 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 999 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200720/01faac17/attachment.bin>
-------------- next part --------------
From 71ea44fd8eef1a84f550eafec54adbe64ca68cd2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 20 Jul 2020 14:36:21 +0100
Subject: [PATCH 1/3] lxd/operations/operations: Renames Operations to Clone
And updates to return a copy of the map rather than an non-current safe reference to the map.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/operations/operations.go | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/lxd/operations/operations.go b/lxd/operations/operations.go
index ffba288981..6b0f3c50b6 100644
--- a/lxd/operations/operations.go
+++ b/lxd/operations/operations.go
@@ -59,9 +59,17 @@ func Unlock() {
operationsLock.Unlock()
}
-// Operations returns a map of operations.
-func Operations() map[string]*Operation {
- return operations
+// Clone returns a clone of the internal operations map containing references to the actual operations.
+func Clone() map[string]*Operation {
+ operationsLock.Lock()
+ defer operationsLock.Unlock()
+
+ localOperations := make(map[string]*Operation, len(operations))
+ for k, v := range operations {
+ localOperations[k] = v
+ }
+
+ return localOperations
}
// OperationGetInternal returns the operation with the given id. It returns an
From f6ab2e407794f95d06b992f47a559e0442080fa4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 20 Jul 2020 14:37:18 +0100
Subject: [PATCH 2/3] lxd-agent/operations: operations.Clone() usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd-agent/operations.go | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/lxd-agent/operations.go b/lxd-agent/operations.go
index 004bb82a28..3c8f033f13 100644
--- a/lxd-agent/operations.go
+++ b/lxd-agent/operations.go
@@ -74,9 +74,7 @@ func operationsGet(d *Daemon, r *http.Request) response.Response {
localOperationURLs := func() (shared.Jmap, error) {
// Get all the operations
- operations.Lock()
- ops := operations.Operations()
- operations.Unlock()
+ ops := operations.Clone()
// Build a list of URLs
body := shared.Jmap{}
@@ -96,9 +94,7 @@ func operationsGet(d *Daemon, r *http.Request) response.Response {
localOperations := func() (shared.Jmap, error) {
// Get all the operations
- operations.Lock()
- ops := operations.Operations()
- operations.Unlock()
+ ops := operations.Clone()
// Build a list of operations
body := shared.Jmap{}
From 11fe9d6debc027ca19f78e60301f4cb536c5e406 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 20 Jul 2020 14:37:41 +0100
Subject: [PATCH 3/3] lxd: operations.Clone() usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/images.go | 2 +-
lxd/operations.go | 12 +++---------
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/lxd/images.go b/lxd/images.go
index a5927a0000..39c7df8d75 100644
--- a/lxd/images.go
+++ b/lxd/images.go
@@ -1635,7 +1635,7 @@ func doImageGet(db *db.Cluster, project, fingerprint string, public bool) (*api.
}
func imageValidSecret(fingerprint string, secret string) (*operations.Operation, bool) {
- for _, op := range operations.Operations() {
+ for _, op := range operations.Clone() {
if op.Resources() == nil {
continue
}
diff --git a/lxd/operations.go b/lxd/operations.go
index 854351ab10..e1815a02ef 100644
--- a/lxd/operations.go
+++ b/lxd/operations.go
@@ -59,9 +59,7 @@ func waitForOperations(s *state.State, chCancel chan struct{}) {
<-tick
// Get all the operations
- operations.Lock()
- ops := operations.Operations()
- operations.Unlock()
+ ops := operations.Clone()
runningOps := 0
@@ -209,9 +207,7 @@ func operationsGet(d *Daemon, r *http.Request) response.Response {
localOperationURLs := func() (shared.Jmap, error) {
// Get all the operations
- operations.Lock()
- localOps := operations.Operations()
- operations.Unlock()
+ localOps := operations.Clone()
// Build a list of URLs
body := shared.Jmap{}
@@ -234,9 +230,7 @@ func operationsGet(d *Daemon, r *http.Request) response.Response {
localOperations := func() (shared.Jmap, error) {
// Get all the operations
- operations.Lock()
- localOps := operations.Operations()
- operations.Unlock()
+ localOps := operations.Clone()
// Build a list of operations
body := shared.Jmap{}
More information about the lxc-devel
mailing list