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

tough: Add locking around datastore write operations #497

Merged
merged 1 commit into from
Nov 8, 2019

Conversation

iliana
Copy link
Contributor

@iliana iliana commented Nov 8, 2019

Issue #, if available: #117 (kinda)

Description of changes:
We don't want separate threads using tough to end up writing to its datastore and reading at the same time. #117 suggested making those operations take &mut self, but that bubbles up to requiring most methods on Repository to also take &mut self.

This adds a RwLock to Datastore, allowing "any number of readers to acquire the lock as long as a writer is not holding the lock".

The difference between this and &mut self is that this is ensured at runtime, which may cause a deadlock (I'm not aware of a way this is possible, because the guard drops out of scope before a reader is even returned), as opposed to ensuring against a data race at compile time, which may just cause borrowck to yell politely at you.

I've tested that this does not, in fact, cause a deadlock in updog.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Member

@jahkeup jahkeup left a comment

Choose a reason for hiding this comment

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

I re-read the description, so I see the sorta-ack on my question. Ship it! 🚢

Copy link
Contributor

@sam-aws sam-aws left a comment

Choose a reason for hiding this comment

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

✔️

@iliana iliana merged commit e1d4edc into develop Nov 8, 2019
@tjkirch tjkirch deleted the tough-datastore-locking branch November 10, 2019 23:53
@iliana iliana added this to the v0.2.0 milestone Nov 19, 2019
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.

4 participants