Hi, Dave,
Attached is a proposed (slightly hacky) patch. The Sendmail::Queue
constructor tests whether or not both fcntl and flock-style locking
work at the same time. If they do, then we use them both whenever we
lock a Qf.
It would be better to have a feature test that tests this once instead
of at run-time, but this is better than nothing.
Regards,
David.
diff -u -r Sendmail-Queue-0.400/MANIFEST Sendmail-Queue-0.600/MANIFEST
--- Sendmail-Queue-0.400/MANIFEST 2010-06-28 14:21:41.000000000 -0400
+++ Sendmail-Queue-0.600/MANIFEST 2012-07-17 08:55:14.000000000 -0400
@@ -17,6 +17,7 @@
META.yml # Will be created by "make dist"
README
t/00-load.t
+t/lock.t
t/pod-coverage.t
t/pod.t
t/sendmail-queue-df.t
diff -u -r Sendmail-Queue-0.400/lib/Sendmail/Queue/Qf.pm Sendmail-Queue-0.600/lib/Sendmail/Queue/Qf.pm
--- Sendmail-Queue-0.400/lib/Sendmail/Queue/Qf.pm 2010-07-20 10:39:49.000000000 -0400
+++ Sendmail-Queue-0.600/lib/Sendmail/Queue/Qf.pm 2012-07-17 08:27:55.000000000 -0400
@@ -12,6 +12,12 @@
use Mail::Header::Generator ();
use Storable ();
+my $fcntl_struct = 's H60';
+my $fcntl_structlockp = pack($fcntl_struct, Fcntl::F_WRLCK,
+ "000000000000000000000000000000000000000000000000000000000000");
+my $fcntl_structunlockp = pack($fcntl_struct, Fcntl::F_UNLCK,
+ "000000000000000000000000000000000000000000000000000000000000");
+
## no critic 'ProhibitMagicNumbers'
# TODO: should we fail if total size of headers > 32768 bytes, or let sendmail die?
@@ -143,10 +149,11 @@
}
}
-=head2 create_and_lock ( )
+=head2 create_and_lock ( [$lock_both] )
Generate a Sendmail 8.12-compatible queue ID, and create a locked qf
-file with that name.
+file with that name. If $lock_both is true, we lock the file using
+both fcntl and flock-style locking.
See Bat Book 3rd edition, section 11.2.1 for information on how the
queue file name is generated.
@@ -164,7 +171,7 @@
sub create_and_lock
{
- my ($self) = @_;
+ my ($self, $lock_both) = @_;
if( ! -d $self->get_queue_directory ) {
die q{Cannot create queue file without queue directory!};
@@ -194,6 +201,13 @@
close($fh);
unlink($path);
$seq = ($seq + 1) % 3600;
+ next;
+ }
+ if ($lock_both && !fcntl($fh, Fcntl::F_SETLK, $fcntl_structlockp)) {
+ # See above... couldn't lock with fcntl
+ close($fh);
+ unlink($path);
+ $seq = ($seq + 1) % 3600;
next;
}
$self->set_queue_id( $qid );
diff -u -r Sendmail-Queue-0.400/lib/Sendmail/Queue.pm Sendmail-Queue-0.600/lib/Sendmail/Queue.pm
--- Sendmail-Queue-0.400/lib/Sendmail/Queue.pm 2010-12-30 14:39:15.000000000 -0500
+++ Sendmail-Queue-0.600/lib/Sendmail/Queue.pm 2012-07-17 08:30:41.000000000 -0400
@@ -4,13 +4,19 @@
use Carp;
use 5.8.0;
-our $VERSION = '0.400';
+our $VERSION = '0.600';
use Sendmail::Queue::Qf;
use Sendmail::Queue::Df;
use File::Spec;
use IO::Handle;
-use Fcntl;
+use Fcntl qw( :flock );
+use File::Temp qw(tempfile);
+my $fcntl_struct = 's H60';
+my $fcntl_structlockp = pack($fcntl_struct, Fcntl::F_WRLCK,
+ "000000000000000000000000000000000000000000000000000000000000");
+my $fcntl_structunlockp = pack($fcntl_struct, Fcntl::F_UNLCK,
+ "000000000000000000000000000000000000000000000000000000000000");
use Sendmail::Queue::Base;
our @ISA = qw( Sendmail::Queue::Base );
@@ -130,6 +136,8 @@
my $self = { queue_directory => $args->{queue_directory}, };
+ $self->{lock_both} = 0;
+
bless $self, $class;
if(!-d $self->{queue_directory}) {
@@ -148,6 +156,17 @@
$self->set_df_directory(File::Spec->catfile($self->{queue_directory}));
}
+ # Check if both fcntl-style and flock-style locking is available
+ my ($fh, $filename) = tempfile();
+ if ($fh) {
+ my $flock_status = flock($fh, LOCK_EX | LOCK_NB);
+ my $fcntl_status = fcntl($fh, Fcntl::F_SETLK, $fcntl_structlockp);
+ if ($flock_status && $fcntl_status) {
+ $self->{lock_both} = 1;
+ }
+ $fh->close();
+ }
+ unlink($filename) if $filename;
return $self;
}
@@ -413,7 +432,7 @@
$cur_qf->set_sender( $sender );
$cur_qf->add_recipient( @{ $env_data->{recipients} } );
- $cur_qf->create_and_lock();
+ $cur_qf->create_and_lock($self->{lock_both});
# As soon as it's created, put it on the list so it can
# be cleaned up later if necessary.
@@ -435,7 +454,21 @@
$first_df->write();
} else {
# Otherwise, link to the first df
- $cur_df->hardlink_to( $first_df->get_data_filename() );
+ eval { $cur_df->hardlink_to( $first_df->get_data_filename() ); };
+ if ($@) {
+ if ($@ =~ /Path .* does not exist/) {
+ # This should NEVER happen...
+ # but it was observed to happen!
+ # Sorry to spew to STDERR, but there's no
+ # feasible way to log this
+ print STDERR 'Sendmail::Queue warning: ' . $first_df->get_data_filename() . ' has disappeared! Writing new file as ' . $cur_df->get_data_filename() . "\n";
+ $first_df = $cur_df;
+ $first_df->set_data($data);
+ $first_df->write();
+ } else {
+ die($@);
+ }
+ }
}
$results{ $env_name } = $cur_qf->get_queue_id;
@@ -481,7 +514,7 @@
# TODO: this needs testing on solaris and bsd
my $directory = $self->get_df_directory();
- sysopen(DIR_FH, $directory, O_RDONLY) or die qq{Couldn't sysopen $directory: $!};
+ sysopen(DIR_FH, $directory, Fcntl::O_RDONLY) or die qq{Couldn't sysopen $directory: $!};
my $handle = IO::Handle->new();
$handle->fdopen(fileno(DIR_FH), 'w') or die qq{Couldn't fdopen the directory handle: $!};
diff -N -u -r Sendmail-Queue-0.400/t/lock.t Sendmail-Queue-0.600/t/lock.t
--- Sendmail-Queue-0.400/t/lock.t 1969-12-31 19:00:00.000000000 -0500
+++ Sendmail-Queue-0.600/t/lock.t 2012-07-17 08:55:01.000000000 -0400
@@ -0,0 +1,55 @@
+package test_lock;
+use strict;
+use warnings;
+
+use base qw(Test::Class);
+
+use Test::Most;
+use File::Temp;
+
+
+use Sendmail::Queue;
+
+my $USER = getpwuid($>);
+
+sub make_tmpdir : Test(setup)
+{
+ my ($self) = @_;
+ $self->{tmpdir} = File::Temp::tempdir( CLEANUP => 1 );
+}
+
+sub del_tmpdir : Test(teardown)
+{
+ my ($self) = @_;
+
+ delete $self->{tmpdir}
+}
+
+sub test_locking : Test(1)
+{
+ my ($self) = @_;
+ my $q = Sendmail::Queue->new({queue_directory => $self->{tmpdir}});
+ my $qf = Sendmail::Queue::Qf->new({
+ queue_directory => $self->{tmpdir}
+ });
+ $qf->set_headers("From: foo\nTo: bar\nSubject: Blech\n");
+ $qf->set_sender('devnull@roaringpenguin.com');
+ $qf->add_recipient('devnull@roaringpenguin.com');
+ $qf->create_and_lock($q->{lock_both});
+ $qf->write();
+ $qf->sync();
+ my $df = Sendmail::Queue::Df->new({
+ queue_directory => $self->{tmpdir},
+ queue_id => $qf->get_queue_id()});
+ $df->set_data("Wookie!\n");
+ $df->write();
+
+ # Now try running sendmail
+ $ENV{'PATH'} .= ":/sbin:/usr/sbin";
+ my $qid = $qf->get_queue_id();
+ my $tmp = $self->{tmpdir};
+ my $output = `sendmail -v -qI$qid -OQueueDirectory=$tmp 2>&1`;
+ like($output, qr/$qid: locked/, 'Queue file was successfully locked according to sendmail');
+}
+
+__PACKAGE__->runtests unless caller();
