Skip to content
This repository was archived by the owner on Mar 27, 2024. It is now read-only.

Fixed concurrent map write in image diffing #278

Merged

Conversation

apourchet
Copy link
Contributor

When running container-diff, I got the following panic due to a race condition:

fatal error: concurrent map writes

goroutine 19 [running]:
runtime.throw(0x15bd053, 0x15)
	/usr/local/go/src/runtime/panic.go:605 +0x95 fp=0xc42006bdd8 sp=0xc42006bdb8 pc=0x102b6f5
runtime.mapassign_faststr(0x15065a0, 0xc42030c090, 0x7ffeefbff90c, 0x11, 0x2)
	/usr/local/go/src/runtime/hashmap_fast.go:783 +0x4f5 fp=0xc42006be58 sp=0xc42006bdd8 pc=0x100d0c5
github.com/GoogleContainerTools/container-diff/cmd.processImage(0x7ffeefbff90c, 0x11, 0xc42030c090, 0xc4202fb5b0, 0xc420262ae0)
	/go/src/github.com/GoogleContainerTools/container-diff/cmd/diff.go:80 +0x2af fp=0xc42006bfb8 sp=0xc42006be58 pc=0x146239f
runtime.goexit()
	/usr/local/go/src/runtime/asm_amd64.s:2337 +0x1 fp=0xc42006bfc0 sp=0xc42006bfb8 pc=0x105bc11
created by github.com/GoogleContainerTools/container-diff/cmd.diffImages
	/go/src/github.com/GoogleContainerTools/container-diff/cmd/diff.go:115 +0x2fc

The variable imageMap was being written to by two different goroutines (processImage) at the same time. This patch fixes that race condition by getting rid of the map altogether and keeping each variable separate.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@apourchet
Copy link
Contributor Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

Copy link
Contributor

@nkubala nkubala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, can't believe I never ran into this error myself! Thanks for fixing @apourchet!

@nkubala nkubala merged commit 9d720eb into GoogleContainerTools:master Dec 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants