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

added locking to mutation methods Store, DeleteOne and DeleteAll #4

Merged
merged 2 commits into from
Nov 15, 2016
Merged

added locking to mutation methods Store, DeleteOne and DeleteAll #4

merged 2 commits into from
Nov 15, 2016

Conversation

nutbunnies
Copy link

Fixes issues with issue mailhog/MailHog#121. Running tests without mutex locks and -race will produce the same fatal error: concurrent map writes error. I'm very new to Go so this might not be the best solution.

@ian-kent
Copy link
Member

👍 thanks @nutbunnies, looks good! if you can fix the mutex unlock thing I'll get it merged 😄

memory.Messages = append(memory.Messages, m)
memory.MessageIDIndex[string(m.ID)] = len(memory.Messages) - 1
memory.mu.Unlock()
Copy link
Member

Choose a reason for hiding this comment

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

this should be a defer memory.mu.Unlock() after line 27 instead, e.g.

memory.mu.Lock()
defer memory.mu.Unlock()

although unlikely, if either line 28 or 29 panics, the mutex would still be locked and would cause all other goroutines to block (see https://gist.github.com/ian-kent/06efa5b60d3b9f54ee07648e770a1958 for an example)

the same applies to the other mutex unlocks in the rest of memory.go

@nutbunnies
Copy link
Author

GTK on the defer stuff; I updated the 3 usages to use defer.

@ian-kent ian-kent merged commit 4266627 into mailhog:master Nov 15, 2016
@ian-kent
Copy link
Member

👍 awesome, thanks @nutbunnies

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

Successfully merging this pull request may close these issues.

2 participants