Hi Peter: Thanks a lot for your feedback!
Am 26.05.17 04:16 schrieb(en) Peter Bloomfield:
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,
Yes, you're right. Good point.
it's object-reffed in the LibBalsaServer code, so there's no need to object-ref it in lbs_process_queue.
No, it think this is necessary. Consider the following (though unlikely) situation: - user triggers a send operation, which in turn starts the availability check - the check takes some time (e.g. a dial-up connection) - the user deletes the server definition, which has no effect due to the ref in libbalsa_server_test_can_reach_full() - the server is reachable, so libbalsa_server_can_reach_cb() calls the callback which launches the sending thread - the server is unred'ef, which will probably lead to a crash in the sending thread. But actually, the unref is misplaced, and would never catch this case - thanks for pointing me to that!
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?
Please find attached ver. 2 of the patch, fixing the issues above. It basically removes the smtp server from SendQueueInfo, but adds it to SendMessageInfo (instead of just its name), so (un) ref'ing it is handled in send_message_info_new() and send_message_info_destroy(). Thanks, Albrecht.
diff --git a/libbalsa/send.c b/libbalsa/send.c
index ffcc56b..f48b2fb 100644
--- a/libbalsa/send.c
+++ b/libbalsa/send.c
@@ -63,20 +63,30 @@ struct _MessageQueueItem {
typedef struct _SendMessageInfo SendMessageInfo;
struct _SendMessageInfo {
+ LibBalsaSmtpServer *smtp_server;
LibBalsaMailbox *outbox;
NetClientSmtp *session;
- gchar *mta_name;
GList *items; /* of MessageQueueItem */
gboolean debug;
};
-static int sending_threads = 0; /* how many sending threads are active? */
+
+typedef struct _SendQueueInfo SendQueueInfo;
+
+struct _SendQueueInfo {
+ LibBalsaMailbox *outbox;
+ LibBalsaFccboxFinder finder;
+ GtkWindow *parent;
+};
+
+
+static gint sending_threads = 0; /* how many sending threads are active, access via g_atomic_* */
/* end of state variables section */
gboolean
libbalsa_is_sending_mail(void)
{
- return sending_threads > 0;
+ return g_atomic_int_get(&sending_threads) > 0;
}
@@ -98,7 +108,7 @@ libbalsa_wait_for_sending_thread(gint max_time)
} else {
max_time *= 1000000; /* convert to microseconds */
}
- while (sending_threads > 0 && sleep_time < max_time) {
+ while ((g_atomic_int_get(&sending_threads) > 0) && (sleep_time < max_time)) {
while (gtk_events_pending()) {
gtk_main_iteration_do(FALSE);
}
@@ -136,16 +146,16 @@ msg_queue_item_destroy(MessageQueueItem *mqi)
static SendMessageInfo *
-send_message_info_new(LibBalsaMailbox *outbox,
- NetClientSmtp *session,
- const gchar *name)
+send_message_info_new(LibBalsaSmtpServer *smtp_server,
+ LibBalsaMailbox *outbox,
+ NetClientSmtp *session)
{
SendMessageInfo *smi;
smi = g_new0(SendMessageInfo, 1);
smi->session = session;
smi->outbox = outbox;
- smi->mta_name = g_strdup(name);
+ smi->smtp_server = g_object_ref(smtp_server);
return smi;
}
@@ -159,7 +169,7 @@ send_message_info_destroy(SendMessageInfo *smi)
if (smi->items != NULL) {
g_list_free(smi->items);
}
- g_free(smi->mta_name);
+ g_object_unref(smi->smtp_server);
g_free(smi);
}
@@ -493,11 +503,13 @@ libbalsa_message_queue(LibBalsaMessage *message,
send the given messsage (if any, it can be NULL) and all the messages
in given outbox.
*/
-static gboolean lbs_process_queue(LibBalsaMailbox *outbox,
- LibBalsaFccboxFinder finder,
- LibBalsaSmtpServer *smtp_server,
- gboolean debug,
- GtkWindow *parent);
+static void lbs_process_queue(LibBalsaMailbox *outbox,
+ LibBalsaFccboxFinder finder,
+ LibBalsaSmtpServer *smtp_server,
+ GtkWindow *parent);
+static void lbs_process_queue_real(LibBalsaSmtpServer *smtp_server,
+ SendQueueInfo *send_info);
+
LibBalsaMsgCreateResult
libbalsa_message_send(LibBalsaMessage *message,
@@ -507,7 +519,6 @@ libbalsa_message_send(LibBalsaMessage *message,
LibBalsaSmtpServer *smtp_server,
GtkWindow *parent,
gboolean flow,
- gboolean debug,
GError **error)
{
LibBalsaMsgCreateResult result = LIBBALSA_MESSAGE_CREATE_OK;
@@ -520,9 +531,8 @@ libbalsa_message_send(LibBalsaMessage *message,
smtp_server, flow, error);
}
- if ((result == LIBBALSA_MESSAGE_CREATE_OK)
- && !lbs_process_queue(outbox, finder, smtp_server, debug, parent)) {
- return LIBBALSA_MESSAGE_SEND_ERROR;
+ if (result == LIBBALSA_MESSAGE_CREATE_OK) {
+ lbs_process_queue(outbox, finder, smtp_server, parent);
}
return result;
@@ -592,18 +602,53 @@ send_message_data_cb(gchar *buffer,
}
+static void
+lbs_check_reachable_cb(GObject *object,
+ gboolean can_reach,
+ gpointer cb_data)
+{
+ LibBalsaSmtpServer *smtp_server = LIBBALSA_SMTP_SERVER(object);
+ SendQueueInfo *send_info = (SendQueueInfo *) cb_data;
+
+ if (can_reach) {
+ lbs_process_queue_real(smtp_server, send_info);
+ } else {
+ libbalsa_information(LIBBALSA_INFORMATION_WARNING,
+ _("Cannot reach SMTP server %s (%s), any queued message will remain in %s."),
+ libbalsa_smtp_server_get_name(smtp_server),
+ LIBBALSA_SERVER(smtp_server)->host,
+ send_info->outbox->name);
+ }
+
+ g_object_unref(send_info->outbox);
+ g_free(send_info);
+}
+
+
+static void
+lbs_process_queue(LibBalsaMailbox *outbox,
+ LibBalsaFccboxFinder finder,
+ LibBalsaSmtpServer *smtp_server,
+ GtkWindow *parent)
+{
+ SendQueueInfo *send_info;
+
+ send_info = g_new(SendQueueInfo, 1U);
+ send_info->outbox = g_object_ref(outbox);
+ send_info->finder = finder;
+ send_info->parent = parent;
+ libbalsa_server_test_can_reach(LIBBALSA_SERVER(smtp_server), lbs_check_reachable_cb, send_info);
+}
+
+
/* libbalsa_process_queue:
treats given mailbox as a set of messages to send. Loads them up and
launches sending thread/routine.
NOTE that we do not close outbox after reading. send_real/thread message
handler does that.
*/
-static gboolean
-lbs_process_queue(LibBalsaMailbox *outbox,
- LibBalsaFccboxFinder finder,
- LibBalsaSmtpServer *smtp_server,
- gboolean debug,
- GtkWindow *parent)
+static void
+lbs_process_queue_real(LibBalsaSmtpServer *smtp_server, SendQueueInfo *send_info)
{
LibBalsaServer *server = LIBBALSA_SERVER(smtp_server);
SendMessageInfo *send_message_info;
@@ -612,9 +657,9 @@ lbs_process_queue(LibBalsaMailbox *outbox,
g_mutex_lock(&send_messages_lock);
- if (!libbalsa_mailbox_open(outbox, NULL)) {
+ if (!libbalsa_mailbox_open(send_info->outbox, NULL)) {
g_mutex_unlock(&send_messages_lock);
- return FALSE;
+ return;
}
/* create the SMTP session */
@@ -636,7 +681,7 @@ lbs_process_queue(LibBalsaMailbox *outbox,
server->cert_file, error->message);
g_error_free(error);
g_mutex_unlock(&send_messages_lock);
- return FALSE;
+ return;
}
}
@@ -645,22 +690,22 @@ lbs_process_queue(LibBalsaMailbox *outbox,
g_signal_connect(G_OBJECT(session), "auth", G_CALLBACK(libbalsa_server_get_auth), smtp_server);
send_message_info =
- send_message_info_new(outbox, session, libbalsa_smtp_server_get_name(smtp_server));
+ send_message_info_new(smtp_server, send_info->outbox, session);
- for (msgno = libbalsa_mailbox_total_messages(outbox); msgno > 0U; msgno--) {
+ for (msgno = libbalsa_mailbox_total_messages(send_info->outbox); msgno > 0U; msgno--) {
MessageQueueItem *new_message;
LibBalsaMessage *msg;
const gchar *smtp_server_name;
LibBalsaMsgCreateResult created;
/* Skip this message if it either FLAGGED or DELETED: */
- if (!libbalsa_mailbox_msgno_has_flags(outbox, msgno, 0,
+ if (!libbalsa_mailbox_msgno_has_flags(send_info->outbox, msgno, 0,
(LIBBALSA_MESSAGE_FLAG_FLAGGED |
LIBBALSA_MESSAGE_FLAG_DELETED))) {
continue;
}
- msg = libbalsa_mailbox_get_message(outbox, msgno);
+ msg = libbalsa_mailbox_get_message(send_info->outbox, msgno);
if (!msg) { /* error? */
continue;
}
@@ -676,7 +721,7 @@ lbs_process_queue(LibBalsaMailbox *outbox,
}
msg->request_dsn = (atoi(libbalsa_message_get_user_header(msg, "X-Balsa-DSN")) != 0);
- new_message = msg_queue_item_new(finder);
+ new_message = msg_queue_item_new(send_info->finder);
created = libbalsa_fill_msg_queue_item_from_queu(msg, new_message);
libbalsa_message_body_unref(msg);
@@ -739,8 +784,8 @@ lbs_process_queue(LibBalsaMailbox *outbox,
if (send_message_info->items != NULL) {
GThread *send_mail;
- ensure_send_progress_dialog(parent);
- sending_threads++;
+ ensure_send_progress_dialog(send_info->parent);
+ g_atomic_int_inc(&sending_threads);
send_mail = g_thread_new("balsa_send_message_real",
(GThreadFunc) balsa_send_message_real,
send_message_info);
@@ -750,26 +795,21 @@ lbs_process_queue(LibBalsaMailbox *outbox,
}
g_mutex_unlock(&send_messages_lock);
- return TRUE;
+ return;
}
-gboolean
+void
libbalsa_process_queue(LibBalsaMailbox *outbox,
LibBalsaFccboxFinder finder,
GSList *smtp_servers,
- GtkWindow *parent,
- gboolean debug)
+ GtkWindow *parent)
{
for (; smtp_servers; smtp_servers = smtp_servers->next) {
LibBalsaSmtpServer *smtp_server =
LIBBALSA_SMTP_SERVER(smtp_servers->data);
- if (!lbs_process_queue(outbox, finder, smtp_server, debug, parent)) {
- return FALSE;
- }
+ lbs_process_queue(outbox, finder, smtp_server, parent);
}
-
- return TRUE;
}
@@ -807,7 +847,7 @@ balsa_send_message_real(SendMessageInfo *info)
GList *this_msg;
gchar *msg;
- msg = g_strdup_printf(_("Connected to MTA %s: %s"), info->mta_name, greeting);
+ msg = g_strdup_printf(_("Connected to SMTP server %s: %s"), libbalsa_smtp_server_get_name(info->smtp_server), greeting);
MSGSENDTHREAD(threadmsg, MSGSENDTHREADPROGRESS, msg, NULL, NULL, 0);
g_free(msg);
for (this_msg = info->items; this_msg != NULL; this_msg = this_msg->next) {
@@ -885,7 +925,7 @@ balsa_send_message_real(SendMessageInfo *info)
} else {
libbalsa_information(LIBBALSA_INFORMATION_ERROR,
_("Connecting MTA %s (%s) failed: %s"),
- info->mta_name,
+ libbalsa_smtp_server_get_name(info->smtp_server),
net_client_get_host(NET_CLIENT(info->session)),
error->message);
g_error_free(error);
@@ -898,10 +938,8 @@ balsa_send_message_real(SendMessageInfo *info)
/* clean up */
send_message_info_destroy(info);
- g_mutex_lock(&send_messages_lock);
MSGSENDTHREAD(threadmsg, MSGSENDTHREADFINISHED, "", NULL, NULL, 0);
- sending_threads--;
- g_mutex_unlock(&send_messages_lock);
+ (void) g_atomic_int_dec_and_test(&sending_threads);
return result;
}
diff --git a/libbalsa/send.h b/libbalsa/send.h
index c7ce3dd..0248e29 100644
--- a/libbalsa/send.h
+++ b/libbalsa/send.h
@@ -65,13 +65,11 @@ LibBalsaMsgCreateResult libbalsa_message_send(LibBalsaMessage * message,
smtp_server,
GtkWindow * parent,
gboolean flow,
- gboolean debug,
GError ** error);
-gboolean libbalsa_process_queue(LibBalsaMailbox * outbox,
- LibBalsaFccboxFinder finder,
- GSList * smtp_servers,
- GtkWindow * parent,
- gboolean debug);
+void libbalsa_process_queue(LibBalsaMailbox * outbox,
+ LibBalsaFccboxFinder finder,
+ GSList * smtp_servers,
+ GtkWindow * parent);
extern GMutex send_messages_lock;
extern int send_thread_pipes[2];
diff --git a/libbalsa/server.c b/libbalsa/server.c
index 5711829..be908d8 100644
--- a/libbalsa/server.c
+++ b/libbalsa/server.c
@@ -720,7 +720,8 @@ libbalsa_server_test_can_reach_full(LibBalsaServer * server,
GObject * source_object)
{
CanReachInfo *info;
- const gchar *host;
+ gchar *host;
+ gchar *colon;
GNetworkMonitor *monitor;
GSocketConnectable *address;
@@ -731,8 +732,13 @@ libbalsa_server_test_can_reach_full(LibBalsaServer * server,
monitor = g_network_monitor_get_default();
- host = server->host;
+ host = g_strdup(server->host);
+ colon = strchr(host, ':');
+ if (colon != NULL) {
+ colon[0] = '\0';
+ }
address = g_network_address_new(host, 0);
+ g_free(host);
g_network_monitor_can_reach_async(monitor, address, NULL,
libbalsa_server_can_reach_cb, info);
g_object_unref(address);
diff --git a/src/balsa-message.c b/src/balsa-message.c
index 292c088..57a62b2 100644
--- a/src/balsa-message.c
+++ b/src/balsa-message.c
@@ -2479,7 +2479,7 @@ handle_mdn_request(GtkWindow *parent, LibBalsaMessage *message)
balsa_find_sentbox_by_url,
mdn_ident->smtp_server,
parent,
- TRUE, balsa_app.debug, &error);
+ TRUE, &error);
if (result != LIBBALSA_MESSAGE_CREATE_OK)
libbalsa_information(LIBBALSA_INFORMATION_ERROR,
_("Sending the disposition notification failed: %s"),
@@ -2620,7 +2620,7 @@ mdn_dialog_response(GtkWidget * dialog, gint response, gpointer user_data)
mdn_ident->smtp_server,
gtk_window_get_transient_for
((GtkWindow *) dialog),
- TRUE, balsa_app.debug, &error);
+ TRUE, &error);
if (result != LIBBALSA_MESSAGE_CREATE_OK)
libbalsa_information(LIBBALSA_INFORMATION_ERROR,
_("Sending the disposition notification failed: %s"),
diff --git a/src/balsa-mime-widget-message.c b/src/balsa-mime-widget-message.c
index d3bf3b9..b98191b 100644
--- a/src/balsa-mime-widget-message.c
+++ b/src/balsa-mime-widget-message.c
@@ -380,7 +380,7 @@ extbody_send_mail(GtkWidget * button, LibBalsaMessageBody * mime_body)
balsa_app.current_ident->smtp_server,
GTK_WINDOW(gtk_widget_get_toplevel
(button)),
- FALSE, balsa_app.debug, &err);
+ FALSE, &err);
if (result != LIBBALSA_MESSAGE_CREATE_OK)
libbalsa_information(LIBBALSA_INFORMATION_ERROR,
_("Sending the external body request failed: %s"),
diff --git a/src/balsa-mime-widget-vcalendar.c b/src/balsa-mime-widget-vcalendar.c
index e0b651f..4438fe0 100644
--- a/src/balsa-mime-widget-vcalendar.c
+++ b/src/balsa-mime-widget-vcalendar.c
@@ -327,7 +327,7 @@ vevent_reply(GObject * button, GtkWidget * box)
ident->smtp_server,
GTK_WINDOW(gtk_widget_get_toplevel
((GtkWidget *) button)),
- FALSE, balsa_app.debug, &error);
+ FALSE, &error);
if (result != LIBBALSA_MESSAGE_CREATE_OK)
libbalsa_information(LIBBALSA_INFORMATION_ERROR,
_("Sending the iTIP calendar reply failed: %s"),
diff --git a/src/main-window.c b/src/main-window.c
index 599a9df..0f58eaa 100644
--- a/src/main-window.c
+++ b/src/main-window.c
@@ -1054,8 +1054,7 @@ send_queued_mail_activated(GSimpleAction * action,
{
libbalsa_process_queue(balsa_app.outbox, balsa_find_sentbox_by_url,
balsa_app.smtp_servers,
- (GtkWindow *) balsa_app.main_window,
- balsa_app.debug);
+ (GtkWindow *) balsa_app.main_window);
}
static void
diff --git a/src/sendmsg-window.c b/src/sendmsg-window.c
index 3518e04..08b5964 100644
--- a/src/sendmsg-window.c
+++ b/src/sendmsg-window.c
@@ -5256,7 +5256,7 @@ send_message_handler(BalsaSendmsg * bsmsg, gboolean queue_only)
balsa_find_sentbox_by_url,
bsmsg->ident->smtp_server,
GTK_WINDOW(bsmsg->window),
- bsmsg->flow, balsa_app.debug, &error);
+ bsmsg->flow, &error);
if (result == LIBBALSA_MESSAGE_CREATE_OK) {
if (bsmsg->parent_message && bsmsg->parent_message->mailbox
&& !bsmsg->parent_message->mailbox->readonly)
pgpkLrcLg23Mp.pgp
Description: PGP signature
_______________________________________________ balsa-list mailing list [email protected] https://mail.gnome.org/mailman/listinfo/balsa-list
