Merge pull request #278 from apourchet/pourchet/fix-race-condition

Fixed concurrent map write in image diffing

Author: nkubala

cmd/diff.go

@@ -71,13 +71,12 @@ func checkFilenameFlag(_ []string) error {
 }
 
 // processImage is a concurrency-friendly wrapper around getImageForName
-func processImage(imageName string, imageMap map[string]*pkgutil.Image, wg *sync.WaitGroup, errChan chan<- error) {
-	defer wg.Done()
+func processImage(imageName string, errChan chan<- error) *pkgutil.Image {
 	image, err := getImage(imageName)
 	if err != nil {
 		errChan <- fmt.Errorf("error retrieving image %s: %s", imageName, err)
 	}
-	imageMap[imageName] = &image
+	return &image
 }
 
 // collects errors from a channel and combines them
@@ -109,18 +108,24 @@ func diffImages(image1Arg, image2Arg string, diffArgs []string) error {
 
 	logrus.Infof("starting diff on images %s and %s, using differs: %s\n", image1Arg, image2Arg, diffArgs)
 
-	imageMap := map[string]*pkgutil.Image{}
+	var image1, image2 *pkgutil.Image
 	errChan := make(chan error, 2)
 
-	go processImage(image1Arg, imageMap, &wg, errChan)
-	go processImage(image2Arg, imageMap, &wg, errChan)
+	go func() {
+		defer wg.Done()
+		image1 = processImage(image1Arg, errChan)
+	}()
+	go func() {
+		defer wg.Done()
+		image2 = processImage(image2Arg, errChan)
+	}()
 
 	wg.Wait()
 	close(errChan)
 
 	if noCache && !save {
-		defer pkgutil.CleanupImage(*imageMap[image1Arg])
-		defer pkgutil.CleanupImage(*imageMap[image2Arg])
+		defer pkgutil.CleanupImage(*image1)
+		defer pkgutil.CleanupImage(*image2)
 	}
 
 	if err := readErrorsFromChannel(errChan); err != nil {
@@ -129,8 +134,8 @@ func diffImages(image1Arg, image2Arg string, diffArgs []string) error {
 
 	logrus.Info("computing diffs")
 	req := differs.DiffRequest{
-		Image1:    *imageMap[image1Arg],
-		Image2:    *imageMap[image2Arg],
+		Image1:    *image1,
+		Image2:    *image2,
 		DiffTypes: diffTypes}
 	diffs, err := req.GetDiff()
 	if err != nil {
@@ -140,15 +145,15 @@ func diffImages(image1Arg, image2Arg string, diffArgs []string) error {
 
 	if filename != "" {
 		logrus.Info("computing filename diffs")
-		err := diffFile(imageMap[image1Arg], imageMap[image2Arg])
+		err := diffFile(image1, image2)
 		if err != nil {
 			return err
 		}
 	}
 
 	if noCache && save {
-		logrus.Infof("images were saved at %s and %s", imageMap[image1Arg].FSPath,
-			imageMap[image2Arg].FSPath)
+		logrus.Infof("images were saved at %s and %s", image1.FSPath,
+			image2.FSPath)
 	}
 	return nil
 }