[PATCH 13/21] lock: retry on EINTR, improve error reporting

[Eric Wong <e@80x24.org>]

We'll also add a handy ->new function since there's
a bunch of places we just create objects with bless.
---
 lib/PublicInbox/Lock.pm | 52 +++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 23 deletions(-)

diff --git a/lib/PublicInbox/Lock.pm b/lib/PublicInbox/Lock.pm
index 0ee2a8bd..ddaf3312 100644
--- 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