http://bugzilla.spamassassin.org/show_bug.cgi?id=4412
------- Additional Comments From [EMAIL PROTECTED] 2005-08-18 15:42 -------
> Also, what failures did you see that you addressed with the change
> "- avoid 65000 attempts on other than EADDRINUSE failure;"
Sorry for being too sketchy...
> I originally did not check for EIADDRINUSE because I thought that
> if it is something else and it works on trying again on another port,
> then why not just keep on trying? If there is some common failure that
> we would want to die immediately on, then we can check for that.
That is what I had in mind. The reasoning is that if something
unexpected happens, it is better to be reported as soon as possible
and with as much information as possible (the $!), instead of being
possibly masked by further attempts. An imaginary scenario to be avoided
is that some day someone would report that a DNS connect takes enormous
amount of time or allocates much system resources, only to be discovered
that it was due to original problem not being noticed and reported.
The background story is what happened when I initially tried that
code with rev3 patch (I had IPv4 address in resolv.conf, and the
module IO::Socket::INET6 was installed): the 'make test' of SA was
very slow and failing:
PERL_DL_NONLAZY=1 /usr/local/bin/perl "-MExtUtils::Command::MM" "-e"
"test_harness(0, 'blib/lib', 'blib/arch')" t/*.tt/basic_lint ....
Can't use an undefined value as a symbol reference
at ../blib/lib/Mail/SpamAssassin/DnsResolver.pm line 428.
It turned out the "Can't use an undefined value as a symbol reference"
was caused by fileno(undef), because the @fhlist in sub fhs_to_vec
had one element which was undef. Tracking this back brought me to
sub connect_sock, where the IO::Socket::INET6->new was failing with
EINVAL because of the wrong address family implicitly chosen.
Unfortunately it didn't report the failure, and wasted the whole round
of 64k attempts to not do anything useful but return undef in $sock.
WHICH MADE ME FIX THE ERROR REPORTING FIRST. Ignoring status, or not
reporting the full error status is always bad, at least in the long run,
when one has to deal with hordes of users, and remote troubleshooting
is very hard to do.
> And how does your patch avoid that?
By calling die with $! in the message.
> I only see that it checks for EIADDRINUSE and if that's not the error,
> it calls _ipv6_ns_warning which only dies if the error is a problem
> with IPv6 nameserver
... AND THEN if _ipv6_ns_warning does not report and dies by itself,
then the: die "Error creating a DNS resolver socket: $errno";
does this instead.
> The changes I see in the patch are:
>
> 1) Add a Domain argument to the IO::Socket constructor call set to:
> If PeerAddr is in IPv6 family and we have IO::Socket::INET6 set it
> to AF_INET6, else set it to AF_INET
True. Perhaps the Domain attribute can even be left unspecified
if HAS_SOCKET_INET6 is false (just in case if comeone does not even
have the AF_INET6 constant).
> 2) In the loop that looks for an unused port, skip the call to
> _ipv6_ns_warning if $! is EIADDRINUSE because that's the error
> we intend to loop over.
That was secondary. The main point is in reporting the $!
when IO::Socket::INET*->new fails and the $! is not some
expected status like EIADDRINUSE, which we know how to deal with.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
