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

Use database locking (flock) for zsh #199

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mafredri
Copy link

@mafredri mafredri commented Mar 1, 2017

This PR adds support for locking the database (via advisory file locking) during update when zsh is used as shell. This is done via zsystem flock that is available in the zsh/system module. The lock is acquired before the database is read and kept until the (new) database has been written and the command has exited.

A separate code path is needed when updating the database entry because replacing the file (via mv) would cause any process waiting for a lock to fail. Here we simply clobber the database with the contents of the tempfile.

This is a partial fix for #198, more specifically, this only fixes the problem on zsh.

// ping @balta2ar

@mafredri mafredri mentioned this pull request Apr 10, 2017
@balta2ar
Copy link

@rupa ping. Could you please review and merge this? I've been using it for two months and so far had no races and DB corruptions.

@balta2ar
Copy link

@rupa ping

1 similar comment
@balta2ar
Copy link

@rupa ping

@jordanlewis
Copy link

Yes please! Would love to see this get merged.

@balta2ar
Copy link

balta2ar commented Jun 6, 2018

@rupa gentle ping

@alphaCTzo7G
Copy link

This would be very useful..

agkozak added a commit to agkozak/zsh-z that referenced this pull request Dec 13, 2018
agkozak added a commit to agkozak/zsh-z that referenced this pull request Dec 13, 2018
agkozak added a commit to agkozak/zsh-z that referenced this pull request Dec 13, 2018
agkozak added a commit to agkozak/zsh-z that referenced this pull request Dec 13, 2018
agkozak added a commit to agkozak/zsh-z that referenced this pull request Dec 13, 2018
@mafredri
Copy link
Author

As this PR is coming up on two years, I'm closing it. If there's anything I can do to make it land, I'd be happy to fix. For example, if the use of mktemp is a problem, it would not be needed if we also landed #247.

@mafredri mafredri closed this Feb 24, 2019
@rupa rupa reopened this Mar 21, 2019
@rupa
Copy link
Owner

rupa commented Mar 21, 2019

sorry my friend, I haven't have a lot of bandwidth, I don't use zsh, and I am definitely slow on changes with this project for a variety of reasons. This is an issue though, and i'd prefer it stay open.

Thanks, and apologies

This commit ensures the database is locked before reading and updating
when used in zsh. We load the zsh/system module which provides the
zsystem flock command to perform advisory file locking.

A separate code path is needed when updating the database entry because
replacing the file (via mv) would cause any process waiting for a lock
to fail. Here we simply clobber the database with the contents of the
tempfile.
@mafredri
Copy link
Author

@rupa that's quite alright, I understand 😄. Rebased on master to get rid of the conflicts.

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.

5 participants