From 975e5b07233b02fbf41990f05f44d76c4584fd51 Mon Sep 17 00:00:00 2001
From: Aaron Lehmann <aaron.lehmann@docker.com>
Date: Mon, 10 Jul 2017 10:05:14 -0700
Subject: [PATCH] container: Abort transactions when memdb calls fail

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
Upstream-commit: bc3209bc156fc5a5bc6e76e5f79a64c60e9a5f7b
Component: engine
---
 components/engine/container/view.go      | 43 ++++++++++++------------
 components/engine/container/view_test.go |  4 +--
 2 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/components/engine/container/view.go b/components/engine/container/view.go
index 449cade149..e6055cc932 100644
--- a/components/engine/container/view.go
+++ b/components/engine/container/view.go
@@ -66,7 +66,7 @@ type ViewDB interface {
 	Delete(*Container) error
 
 	ReserveName(name, containerID string) error
-	ReleaseName(name string)
+	ReleaseName(name string) error
 }
 
 // View can be used by readers to avoid locking
@@ -150,28 +150,20 @@ func (db *memDB) Save(c *Container) error {
 // Delete removes an item by ID
 func (db *memDB) Delete(c *Container) error {
 	txn := db.store.Txn(true)
-	defer txn.Commit()
 
-	// Delete any names referencing this container's ID.
-	iter, err := txn.Get(memdbNamesTable, memdbContainerIDIndex, c.ID)
-	if err != nil {
-		return err
-	}
-
-	var names []string
-	for {
-		item := iter.Next()
-		if item == nil {
-			break
-		}
-		names = append(names, item.(nameAssociation).name)
-	}
+	view := &memdbView{txn: txn}
+	names := view.getNames(c.ID)
 
 	for _, name := range names {
 		txn.Delete(memdbNamesTable, nameAssociation{name: name})
 	}
 
-	return txn.Delete(memdbContainersTable, NewBaseContainer(c.ID, c.Root))
+	if err := txn.Delete(memdbContainersTable, NewBaseContainer(c.ID, c.Root)); err != nil {
+		txn.Abort()
+		return err
+	}
+	txn.Commit()
+	return nil
 }
 
 // ReserveName registers a container ID to a name
@@ -180,29 +172,38 @@ func (db *memDB) Delete(c *Container) error {
 // A name reservation is globally unique
 func (db *memDB) ReserveName(name, containerID string) error {
 	txn := db.store.Txn(true)
-	defer txn.Commit()
 
 	s, err := txn.First(memdbNamesTable, memdbIDIndex, name)
 	if err != nil {
+		txn.Abort()
 		return err
 	}
 	if s != nil {
+		txn.Abort()
 		if s.(nameAssociation).containerID != containerID {
 			return ErrNameReserved
 		}
 		return nil
 	}
 
-	txn.Insert(memdbNamesTable, nameAssociation{name: name, containerID: containerID})
+	if err := txn.Insert(memdbNamesTable, nameAssociation{name: name, containerID: containerID}); err != nil {
+		txn.Abort()
+		return err
+	}
+	txn.Commit()
 	return nil
 }
 
 // ReleaseName releases the reserved name
 // Once released, a name can be reserved again
-func (db *memDB) ReleaseName(name string) {
+func (db *memDB) ReleaseName(name string) error {
 	txn := db.store.Txn(true)
-	txn.Delete(memdbNamesTable, nameAssociation{name: name})
+	if err := txn.Delete(memdbNamesTable, nameAssociation{name: name}); err != nil {
+		txn.Abort()
+		return err
+	}
 	txn.Commit()
+	return nil
 }
 
 type memdbView struct {
diff --git a/components/engine/container/view_test.go b/components/engine/container/view_test.go
index 2e81998ca4..09ba343830 100644
--- a/components/engine/container/view_test.go
+++ b/components/engine/container/view_test.go
@@ -114,7 +114,7 @@ func TestNames(t *testing.T) {
 	assert.EqualError(t, db.ReserveName("name2", "containerid3"), ErrNameReserved.Error())
 
 	// Releasing a name allows the name to point to something else later.
-	db.ReleaseName("name2")
+	assert.NoError(t, db.ReleaseName("name2"))
 	assert.NoError(t, db.ReserveName("name2", "containerid3"))
 
 	view := db.Snapshot()
@@ -131,7 +131,7 @@ func TestNames(t *testing.T) {
 	assert.EqualError(t, err, ErrNameNotReserved.Error())
 
 	// Releasing and re-reserving a name doesn't affect the snapshot.
-	db.ReleaseName("name2")
+	assert.NoError(t, db.ReleaseName("name2"))
 	assert.NoError(t, db.ReserveName("name2", "containerid4"))
 
 	id, err = view.GetID("name1")