Re^4: Race condition with Mail::Sender::MailMsg?

[DrewP]

Just to confirm the suggested solution works. Today, just at the time the internet connection went down, my script logged the expiry of a 20 second timeout sending a simple message but carried on running.

Of course it's easy for me because I know what sort of messages I will be sending and can thus set appropriate timeouts. As a general solution the calculation of the appropriate timeout becomes more complicated.

In case it is of interest to anyone I have put my modified code below.

sub sendmail {
  my ($arg_ref) = @_;
  print "Running sendmail()\n" if ($onterm && ($debug & $DEBUG_NOTIFY_
+MAILS));

=pod
 This subroutine is the interface to the mail sender functionality.
 Depending on incoming parameters it sends just a message or a message
+ with attachment
 If the attachment file does not exist it sends message with notificat
+ion that file was not found


=cut
  my $sendfailed = 0;
  my $sender = new Mail::Sender( {from =>  $arg_ref->{from},
                  smtp => $arg_ref->{smtp},
                  port => $arg_ref->{port},
                  auth => $arg_ref->{auth},
                  authid =>  $arg_ref->{authid},
                  authpwd => $arg_ref->{authpwd},
                  auth_encoded => $arg_ref->{auth_encoded},
                  encoding => 'Quoted-printable',
                  charset => $charset,
                 });

  if ($sender > 0) {
    print "\$sender created OK\n" if ($onterm && ($debug & $DEBUG_NOTI
+FY_MAILS));

    if ((defined $arg_ref->{file}) && (-f  $arg_ref->{file})) {  # Sen
+d message with attachment if the attached file exists
      print "sending mail with attachment\n" if ($onterm && ($debug & 
+$DEBUG_NOTIFY_MAILS));
      my $mailfiletimeout = 120;  #120 seconds
      
      eval {
    local $SIG{ALRM} = sub { die "MailFile Timeout\n" }; # NB: \n requ
+ired
    alarm $mailfiletimeout;
    
    $sender->MailFile({to => $arg_ref->{to},
               subject => $arg_ref->{subject},
               encoding => "Base64", ## read this from parameters?
               msg => encode($charset,$arg_ref->{msg}),
               file => $arg_ref->{file}
              });
    alarm 0;
      };

      if ($@) {
    die unless $@ eq "MailFile Timeout\n"; # propagate unexpected erro
+rs
    logerror("Timeout ($mailfiletimeout seconds) sending $arg_ref->{fi
+le} to $arg_ref->{to} via $arg_ref->{smtp}");
    return 0;
      }
      else {
    if (defined $sender->{'error'} ){
      logerror( "Error sending mail with attachment '$arg_ref->{file}'
+ to $arg_ref->{to}: Error code $sender->{'error'} ($sender->{'error_m
+sg'})");
      $sendfailed = 1;
    }
    else {
      loginfo("File \"$arg_ref->{file}\" successfully emailed to $arg_
+ref->{to}");
    }
      }
    }
    else {  #send a message with no attachment
      #Should never receive a filename for a file that does not exist.
+  Add a line to the message if it ever happens
      $arg_ref->{msg} .= "\n\nAttempt to send file  '$arg_ref->{file}'
+ abandoned: File does not exist" if defined $arg_ref->{file};
      print "sending mail without attachment\n" if ($onterm && ($debug
+ & $DEBUG_NOTIFY_MAILS));
      my $mailmessagetimeout = 20;  #20 seconds
      eval {
    local $SIG{ALRM} = sub { die "MailMsg Timeout\n" }; # NB: \n requi
+red
    alarm $mailmessagetimeout;
    $sender->MailMsg({to => $arg_ref->{to},
              subject => $arg_ref->{subject},
              encoding => "Quoted-printable", ## read this from parame
+ters?
              msg => encode($charset,$arg_ref->{msg}),
             });
    alarm 0;
      };
      if ($@) {
    die unless $@ eq "MailMsg Timeout\n"; # propagate unexpected error
+s
    logerror("Timeout ($mailmessagetimeout seconds) sending message to
+ $arg_ref->{to} via $arg_ref->{smtp}");
    return 0;
      }
      else {
    if (defined $sender->{'error'} ){
      logerror( "Error sending mail to $arg_ref->{to}: Error code $sen
+der->{'error'} ($sender->{'error_msg'})");
      $sendfailed = 1;
    }
    else {
      print "Mail sent to  $arg_ref->{to}\n" if  ($onterm && ($debug &
+ $DEBUG_NOTIFY_MAILS));
    }
      }
    } # send a message with no attachment
  } #  if ($sender > 0) 
  else {
    logerror("Could not establish mail sender to $arg_ref->{to}: Error
+ code $sender ($Mail::Sender::Error)");
    $sendfailed = 1;
  }
  print "Leaving sendmail (\$sendfailed = $sendfailed)\n" if  ($onterm
+ && ($debug & $DEBUG_NOTIFY_MAILS));
  return ($sendfailed) ? 0 : 1;
}

[Marshall]

Thanks for posting your working code!
some minor suggestions:

....
eval 
{
    local $SIG{ALRM} = sub { die "MailFile Timeout\n" }; 
    alarm $mailfiletimeout; #20 seconds
    
    $sender->MailFile(...);
    alarm 0;
};

if ($@) 
{
   die unless $@ eq "MailFile Timeout\n"; # propagate 
                                          # unexpected errors
   logerror("Timeout ....);
   return 0;
}

# no else{} is required, what "else" ????
# Simplify the logic - reduduce levels of indendation
# if there is a serious error we don't get here...
# there is no "else"

# proceed with more code...