lock: retry on EINTR, improve error reporting

We'll also add a handy ->new function since there's
a bunch of places we just create objects with bless.

diff --git a/lib/PublicInbox/Lock.pm b/lib/PublicInbox/Lock.pm
index 0ee2a8bd60bd964a13c0c24f958f43e0ca9a6b5b..ddaf3312ade566b50b6d051b32772c846107337b 100644 (file)
--- a/lib/PublicInbox/Lock.pm
+++ b/lib/PublicInbox/Lock.pm
@@ -1,36 +1,42 @@
-# Copyright (C) 2018-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
-# Base class for per-inbox locking
+# Base class for per-inbox locking, subclassed by several
+# only uses {lock_path} and {lockfh} fields
 package PublicInbox::Lock;
-use strict;
-use v5.10.1;
-use Fcntl qw(:flock :DEFAULT);
+use v5.12;
+use Fcntl qw(LOCK_UN LOCK_EX O_RDWR O_CREAT);
 use Carp qw(croak);
 use PublicInbox::OnDestroy;
+use Errno qw(EINTR);
+use autodie qw(close sysopen syswrite);
+
+sub xflock ($$) {
+       until (flock($_[0], $_[1])) { return if $! != EINTR }
+       1;
+}
+
+sub new { bless { lock_path => $_[1] }, $_[0] }
 
 # we only acquire the flock if creating or reindexing;
 # PublicInbox::Import already has the lock on its own.
 sub lock_acquire {
        my ($self) = @_;
-       my $lock_path = $self->{lock_path};
-       croak 'already locked '.($lock_path // '(undef)') if $self->{lockfh};
-       return unless defined($lock_path);
-       sysopen(my $lockfh, $lock_path, O_RDWR|O_CREAT) or
-               croak "failed to open $lock_path: $!\n";
-       flock($lockfh, LOCK_EX) or croak "lock $lock_path failed: $!\n";
-       $self->{lockfh} = $lockfh;
+       my $fn = $self->{lock_path};
+       croak 'already locked '.($fn // '(undef)') if $self->{lockfh};
+       $fn // return;
+       sysopen(my $fh, $fn, O_RDWR|O_CREAT);
+       xflock($fh, LOCK_EX) or croak "LOCK_EX $fn: $!";
+       $self->{lockfh} = $fh;
 }
 
 sub lock_release {
        my ($self, $wake) = @_;
-       defined(my $lock_path = $self->{lock_path}) or return;
-       my $lockfh = delete $self->{lockfh} or croak "not locked: $lock_path";
-
-       syswrite($lockfh, '.') if $wake;
-
-       flock($lockfh, LOCK_UN) or croak "unlock $lock_path failed: $!\n";
-       close $lockfh or croak "close $lock_path failed: $!\n";
+       my $fn = $self->{lock_path} // return;
+       my $fh = delete $self->{lockfh} or croak "not locked: $fn";
+       syswrite($fh, '.') if $wake;
+       xflock($fh, LOCK_UN) or croak "LOCK_UN $fn: $!";
+       close $fh; # may detect errors
 }
 
 # caller must use return value
@@ -41,13 +47,13 @@ sub lock_for_scope {
 }
 
 sub lock_acquire_fast {
-       $_[0]->{lockfh} or return lock_acquire($_[0]);
-       flock($_[0]->{lockfh}, LOCK_EX) or croak "lock (fast) failed: $!";
+       my $fh = $_[0]->{lockfh} or return lock_acquire($_[0]);
+       xflock($fh, LOCK_EX) or croak "LOCK_EX $_[0]->{lock_path}: $!";
 }
 
 sub lock_release_fast {
-       flock($_[0]->{lockfh} // return, LOCK_UN) or
-                       croak "unlock (fast) $_[0]->{lock_path}: $!";
+       xflock($_[0]->{lockfh} // return, LOCK_UN) or
+               croak "LOCK_UN $_[0]->{lock_path}: $!"
 }
 
 # caller must use return value