[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [msmtp-users] Bug in msmtpq might cause mail loss



Hi Phillip!

On 27/10/11 20:27, Philipp Hartwig wrote:
> I want to add that there is also an ugly race condition in the queue script 
> coming from the way the queue is locked. I run into this error reproducibly 
> when bouncing several messages simultaneously in mutt, so it is not 
> esoteric. It results in errors such as
> 
> 2011 26 Oct 21:07:27 : preparing to send .mail file [ /home/ph/.msmtpqueue/2011-10-26-21.07.27.mail ] but
> 2011 26 Oct 21:07:27 :   corresponding .msmtp file [ /home/ph/.msmtpqueue/2011-10-26-21.07.27.msmtp ] was not found in queue
> 2011 26 Oct 21:07:27 :   skipping this mail ; this is worth looking into
>  
> I've attached two scripts which illustrate the problem, modeled on [1]. Go to an 
> *EMPTY* directory, copy the scripts there and run
> $ echo 0 > testcounterfile; for (( i=1; i <= 10; i++ )); do ./testscript2.sh&; done
> This launches 10 instances of testscript2.sh each of which has the duty to 
> increment the number in testcounterfile by 1. The fact that testcounterfile 
> reproducibly contains numbers strictly smaller than 10 after all instances 
> of testscript2.sh have terminated shows that the locking doesn't work 
> reliably.
> 
> The issue is that another instance of the script can squeeze between
>     [ -e "$LOK" ]
> and
>     touch "$LOK"
> 
> One solution is to use mkdir and check the exit status because mkdir is 
> atomic. The testscript3.sh illustrates this and with this I reliably get 
> the value 10 in testcounterfile when executing
> $ echo 0 > testcounterfile; for (( i=1; i <= 10; i++ )); do ./testscript3.sh&; done
> 
> I've attached a patch which implements the behavior of testscript3.sh 
> msmtpq. Any thoughts?

Thank you very much for working on this and for your detailed analysis.

Both of your proposed patches fix serious problems in the msmtpq script,
so now I applied and pushed them both even though we did not yet receive
a comment from the script's maintainer:
http://msmtp.git.sourceforge.net/git/gitweb.cgi?p=msmtp/msmtp;a=commitdiff;h=454c1fe81e73b688dc755828986277b133df671f
http://msmtp.git.sourceforge.net/git/gitweb.cgi?p=msmtp/msmtp;a=commitdiff;h=02cefc937b4b62a5071cee9407d93d3edfb6b1a8

Chris, if you do not agree to these patches, please let us know.

Martin