Re: Testing network availability

[Peter Bloomfield <PeterBloomfield bellsouth net>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Albrecht:

On 05/25/2017 12:07:15 PM Thu, Albrecht Dreß wrote:
> Hi all!
>
> Am 23.05.17 00:58 schrieb(en) Peter Bloomfield:
> > The git "master" branch has some new code, for more careful checking of when a POP3 host can be reached. This 
> > should avert some warnings about failure to connect to POP3 hosts. Please report any issues with it!
>
> As a follow-up to this patch, I implemented the same feature for outgoing smtp connections, using Peter's new 
> functions.
>
> Basically, when a specific smtp server is not reachable, a warning is shown, and the messages are left in the outbox.  
> Unlike after an error, the messages are /not/ tagged.  Therefore, simply choosing "Send queued mail" will 
> send the messages if possible, or just leave them (untagged) in the outbox if the server is still unreachable.  I did 
> not implement some kind of timer to send queued messages automatically.
>
> In libbalsa/send.c I noticed that the sending_threads global variable is used in an unsafe way as it is 
> accessed from multiple threads without always using a mutex.  The simple fix is to use atomic operations.  
> I'm not sure if the mutex send_messages_lock is actually needed, but it doesn't hurt IMO. BTW, I also ref the 
> outbox and smtp_server objects before calling libbalsa_server_test_can_reach() which is somewhat 
> extra-paranoid, but shouldn't be an issue, too.
>
> In Peter's implementation, the function libbalsa_server_test_can_reach_full() will always report an unreachable server 
> if the host contains a port number ("host:port") which is typical for SMTP.  I also fixed this issue.
>
> Finally, I removed the debug flag from the sending stuff, as debugging in libnetclient is controlled by the 
> G_MESSAGES_DEBUG environment variable.
>
> Opinions?

Thanks for the patch, and the effort you put into it!

One thought: in lbs_check_reachable_cb, the GObject *object argument is actually the LibBalsa *server in the 
call to libbalsa_server_test_can_reach, so including it in SendQueueInfo is redundant; also, it's 
object-reffed in the LibBalsaServer code, so there's no need to object-ref it in lbs_process_queue.

Passing the server explicitly to lbs_process_queue_real complicates the call a little, but if it is cast as a 
(LibBalsaServer *), it would make the first declaration (LibBalsaServer *server = 
LIBBALSA_SERVER(send_info->smtp_server)) redundant.

What do you think?

Peter
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iEYEARECAAYFAlknkBMACgkQH1/UtbkqdPVn5wCbB5FmtLGngsKbXY7xXQ7VNWQf
4WsAn2waPVCBN/ugJ3c9Yk8R82tydFog
=8tem
-----END PGP SIGNATURE-----