Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make snapshots concurrency safe #45

Closed
alexhornbake opened this issue Aug 21, 2018 · 4 comments
Closed

Make snapshots concurrency safe #45

alexhornbake opened this issue Aug 21, 2018 · 4 comments

Comments

@alexhornbake
Copy link
Contributor

I may tackle this one, but want to raise here first.

I think it would be worth converting snapshots map[snapshotID]*snapshot to use sync.Map, or perhaps just use a mutex to protect it. Wondering if there is a preference for minimum golang version that should be targeted, since sync.Map was introduced in golang 1.9. Thoughts?

For context, my use case is a set of tests that assert snapshots from multiple concurrent goroutines. As written, it hits the error: fatal error: concurrent map writes while updating snapshots on github.com/beme/abide/abide.go:230.

@sjkaliski
Copy link
Collaborator

@alexhornbake go for it! 1.9 is a sufficient minimum version.

@sjkaliski
Copy link
Collaborator

@alexhornbake thoughts? Should be ok now that we dropped 1.8.x support.

@sjkaliski
Copy link
Collaborator

@alexhornbake good to close?

@alexhornbake
Copy link
Contributor Author

@sjkaliski yup!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants