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

[Andres P]

On Mon, Jun 21, 2010 at 12:37 PM, Denis A. Altoé Falqueto
<denisfalqueto at gmail.com> wrote:
> On Mon, Jun 21, 2010 at 1:55 PM, Andres P <aepd87 at gmail.com> 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