Re: [pacman-dev] [PATCH 2/2] pacman-optimize: fix database locking race conditions

On Mon, Jun 21, 2010 at 12:37 PM, Denis A. Altoé Falqueto wrote:

On Mon, Jun 21, 2010 at 1:55 PM, Andres P wrote:

...

Instead of making the lock file with touch, use mkdir since it's the only portable atomic transaction available to shell scripts.

In fact, there is another way to do an atomic operation. Look at repo-add script, in function check_repo_db (I've seen this advice in some article some time ago too..). The first command tries to create a file with set -o noclobber. So, if the file already exists, it will fail and will not alter the lock file. If it doesn't exist, the lock file will be created and the process will proceed.

if (set -o noclobber; echo "$$" > "$LOCKFILE") 2>/dev/null; then  # We acquired the lock else  # The lock already exists and we can't proceed. fi

Redirections are not atomic. First of all, echo isn't atomic so the ret val is old. Second, even when using this $ true > lock or simply $ > lock is still not atomic. I realize that this doesn't mean squat until I present my sources, but I will eventually. But the important factor is portability, as I'll explain below.

This would avoid changing the db.lock to a directory. Maybe this could break some kind of script or utility out there...

That would be a bug with that script in particular, because pacman does not care if it's a directory or not, as it should. The lock file in acpid, to name one of the many programs that smartly implement directories as lockfiles, is done this way for a reason. No clobber isn't available in all shells, so you are assuming that only bash/ksh will be interacting with the lockfile. makepkg and similar bash scripts in the source distribution aren't the only ways to interact with pacman. As proof, install scripts are supposed to be sh compatible. So we should keep compat with sh when it comes to something as low level and crucial as locking the db. Andres P