Hi Albrecht, On 07/23/2016 09:54:55 AM Sat, Albrecht Dreß wrote:
Hi all,attached is a large patch which tries to streamline multithreading in Balsa. Currently, the balsa code contains a mixture of glib's thread (and mutex and condition variable) implementation, and the "native" pthread one. Furthermore, multithreading can be disabled completely (why?). Basically, my patch attempts to (a) always use multithreading and (b) only use the glib abstraction layer instead of "native" pthreads. IMO, this simplifies and improves the readability of the code. A special case is the cancel operation (via pthread_cancel()) which GThread does not support. However, I have the feeling that its usage is completely disabled anyway in Balsa: - According to the comment in src/main.c, function balsa_cleanup(), pthread_cancel(get_mail_thread) is called to terminate a pending mail check thread. - This (detached) thread is started from src/main-window.c, function check_new_messages_real(), as function bw_check_messages_thread(). - The very first operation of this function (also in src/main-window.c) is to call pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL), i.e. to block all cancellation requests. The cacellation state is never enabled again. - The only place where pthread_testcancel() is called is in libbalsa/mailbox.c, at the end of the function libbalsa_mailbox_check(), but its completely useless as long as the cancellation state is disabled. So I *think* removing the cancellation stuff is just safe. However, if we need a way (do we?) to cancel a pending slow mailbox check operation when Balsa is closed, we need to implement it differently. To be honest, I have no idea how this could be done properly... The other changes are rather simple - just replace pthread_t by GThread, pthread_mutex_t by GMutex, and pthread_cond_t by GCond, and completely remove the configure option. Please note that both pthread_cond_wait() and g_cond_wait() shall always be called in a loop re-checking the predicate as spurious wakeups may occur [1], so the current implementation is actually wrong in some places. Furthermore, the following minor changes are included: - libbalsa/information.h, libbalsa/libbalsa.h, src/ab-main.c, src/information-dialog.h: use the standard glib way to set gcc function attributes - fix missing parentheses in a condition in function libbalsa_mailbox_messages_change_flags() Opinions? Cheers, Albrecht.
Thanks for a great clean-up effort! It all works for me... As I recall, the option to disable threads has been useful in the past, when the thread implementation had occasional deadlocks; I haven't used a non-threaded build in years--possible a decade! If there's no objection in a day or two, I'll commit it all. Best, Peter
pgpGxxKRvuIPk.pgp
Description: PGP signature
_______________________________________________ balsa-list mailing list [email protected] https://mail.gnome.org/mailman/listinfo/balsa-list
