From c4990ab999d49e261c5079925f0b13ef735a729f Mon Sep 17 00:00:00 2001 From: Alexandr Morozov Date: Thu, 29 May 2014 16:22:13 +0400 Subject: [PATCH] Fix races on TagStore accessing Docker-DCO-1.1-Signed-off-by: Alexandr Morozov (github: LK4D4) --- graph/tags.go | 53 +++++++++++++++++++++++++++++++++++++----------- server/server.go | 15 +++----------- 2 files changed, 44 insertions(+), 24 deletions(-) diff --git a/graph/tags.go b/graph/tags.go index 524e1a1f9d..7af6d383d8 100644 --- a/graph/tags.go +++ b/graph/tags.go @@ -3,13 +3,15 @@ package graph import ( "encoding/json" "fmt" - "github.com/dotcloud/docker/image" - "github.com/dotcloud/docker/utils" "io/ioutil" "os" "path/filepath" "sort" "strings" + "sync" + + "github.com/dotcloud/docker/image" + "github.com/dotcloud/docker/utils" ) const DEFAULTTAG = "latest" @@ -18,6 +20,7 @@ type TagStore struct { path string graph *Graph Repositories map[string]Repository + sync.Mutex } type Repository map[string]string @@ -33,8 +36,8 @@ func NewTagStore(path string, graph *Graph) (*TagStore, error) { Repositories: make(map[string]Repository), } // Load the json file if it exists, otherwise create it. - if err := store.Reload(); os.IsNotExist(err) { - if err := store.Save(); err != nil { + if err := store.reload(); os.IsNotExist(err) { + if err := store.save(); err != nil { return nil, err } } else if err != nil { @@ -43,7 +46,7 @@ func NewTagStore(path string, graph *Graph) (*TagStore, error) { return store, nil } -func (store *TagStore) Save() error { +func (store *TagStore) save() error { // Store the json ball jsonData, err := json.Marshal(store) if err != nil { @@ -55,7 +58,7 @@ func (store *TagStore) Save() error { return nil } -func (store *TagStore) Reload() error { +func (store *TagStore) reload() error { jsonData, err := ioutil.ReadFile(store.path) if err != nil { return err @@ -74,6 +77,8 @@ func (store *TagStore) LookupImage(name string) (*image.Image, error) { tag = DEFAULTTAG } img, err := store.GetImage(repos, tag) + store.Lock() + defer store.Unlock() if err != nil { return nil, err } else if img == nil { @@ -87,6 +92,8 @@ func (store *TagStore) LookupImage(name string) (*image.Image, error) { // Return a reverse-lookup table of all the names which refer to each image // Eg. {"43b5f19b10584": {"base:latest", "base:v1"}} func (store *TagStore) ByID() map[string][]string { + store.Lock() + defer store.Unlock() byID := make(map[string][]string) for repoName, repository := range store.Repositories { for tag, id := range repository { @@ -130,8 +137,10 @@ func (store *TagStore) DeleteAll(id string) error { } func (store *TagStore) Delete(repoName, tag string) (bool, error) { + store.Lock() + defer store.Unlock() deleted := false - if err := store.Reload(); err != nil { + if err := store.reload(); err != nil { return false, err } if r, exists := store.Repositories[repoName]; exists { @@ -150,13 +159,15 @@ func (store *TagStore) Delete(repoName, tag string) (bool, error) { deleted = true } } else { - fmt.Errorf("No such repository: %s", repoName) + return false, fmt.Errorf("No such repository: %s", repoName) } - return deleted, store.Save() + return deleted, store.save() } func (store *TagStore) Set(repoName, tag, imageName string, force bool) error { img, err := store.LookupImage(imageName) + store.Lock() + defer store.Unlock() if err != nil { return err } @@ -169,7 +180,7 @@ func (store *TagStore) Set(repoName, tag, imageName string, force bool) error { if err := validateTagName(tag); err != nil { return err } - if err := store.Reload(); err != nil { + if err := store.reload(); err != nil { return err } var repo Repository @@ -183,11 +194,13 @@ func (store *TagStore) Set(repoName, tag, imageName string, force bool) error { store.Repositories[repoName] = repo } repo[tag] = img.ID - return store.Save() + return store.save() } func (store *TagStore) Get(repoName string) (Repository, error) { - if err := store.Reload(); err != nil { + store.Lock() + defer store.Unlock() + if err := store.reload(); err != nil { return nil, err } if r, exists := store.Repositories[repoName]; exists { @@ -198,6 +211,8 @@ func (store *TagStore) Get(repoName string) (Repository, error) { func (store *TagStore) GetImage(repoName, tagOrID string) (*image.Image, error) { repo, err := store.Get(repoName) + store.Lock() + defer store.Unlock() if err != nil { return nil, err } else if repo == nil { @@ -215,6 +230,20 @@ func (store *TagStore) GetImage(repoName, tagOrID string) (*image.Image, error) return nil, nil } +func (store *TagStore) GetRepoRefs() map[string][]string { + store.Lock() + reporefs := make(map[string][]string) + + for name, repository := range store.Repositories { + for tag, id := range repository { + shortID := utils.TruncateID(id) + reporefs[shortID] = append(reporefs[shortID], fmt.Sprintf("%s:%s", name, tag)) + } + } + store.Unlock() + return reporefs +} + // Validate the name of a repository func validateRepoName(name string) error { if name == "" { diff --git a/server/server.go b/server/server.go index 0deda0d955..b817068369 100644 --- a/server/server.go +++ b/server/server.go @@ -684,15 +684,7 @@ func (srv *Server) ImagesViz(job *engine.Job) engine.Status { } } - reporefs := make(map[string][]string) - - for name, repository := range srv.daemon.Repositories().Repositories { - for tag, id := range repository { - reporefs[utils.TruncateID(id)] = append(reporefs[utils.TruncateID(id)], fmt.Sprintf("%s:%s", name, tag)) - } - } - - for id, repos := range reporefs { + for id, repos := range srv.daemon.Repositories().GetRepoRefs() { job.Stdout.Write([]byte(" \"" + id + "\" [label=\"" + id + "\\n" + strings.Join(repos, "\\n") + "\",shape=box,fillcolor=\"paleturquoise\",style=\"filled,rounded\"];\n")) } job.Stdout.Write([]byte(" base [style=invisible]\n}\n")) @@ -713,6 +705,7 @@ func (srv *Server) Images(job *engine.Job) engine.Status { return job.Error(err) } lookup := make(map[string]*engine.Env) + srv.daemon.Repositories().Lock() for name, repository := range srv.daemon.Repositories().Repositories { if job.Getenv("filter") != "" { if match, _ := path.Match(job.Getenv("filter"), name); !match { @@ -742,6 +735,7 @@ func (srv *Server) Images(job *engine.Job) engine.Status { } } + srv.daemon.Repositories().Unlock() outs := engine.NewTable("Created", len(lookup)) for _, value := range lookup { @@ -1303,9 +1297,6 @@ func (srv *Server) pullRepository(r *registry.Registry, out io.Writer, localName return err } } - if err := srv.daemon.Repositories().Save(); err != nil { - return err - } return nil }