From 123d96aa7703054bcbc51e911d73db7190428d35 Mon Sep 17 00:00:00 2001 From: Matthew Barnes Date: Sun, 12 Aug 2012 10:12:38 -0400 Subject: [PATCH] Rename camel_session_get_service(). Applying lessons learned from ESourceRegistry. Rename camel_session_get_service() to camel_session_ref_service() and have it return a new reference. When returning a pointer to a reference-counted object in a multi- threaded environment, always increase the object's reference count before returning so as to transfer a new reference to the caller. Otherwise it introduces a potential race where the reference-counted object may lose its last reference and be freed while the caller is still using the object. Even if the caller immediately increments the object's reference count, it's still a potential race. Transferring a new reference to the caller means the caller must unreference the object when finished with it so the object will be properly freed when it's no longer needed. Similarly for camel_session_get_service_by_url(). --- camel/camel-filter-search.c | 24 +++++++++++++++++------- camel/camel-sasl-popb4smtp.c | 14 +++++++------- camel/camel-session.c | 25 +++++++++++++++++-------- camel/camel-session.h | 4 ++-- docs/reference/camel/camel-sections.txt | 4 ++-- 5 files changed, 45 insertions(+), 26 deletions(-) diff --git a/camel/camel-filter-search.c b/camel/camel-filter-search.c index dbcc7c6..c38f2d5 100644 --- a/camel/camel-filter-search.c +++ b/camel/camel-filter-search.c @@ -605,14 +605,14 @@ get_relative_months (struct _CamelSExp *f, } static CamelService * -get_service_for_source (CamelSession *session, +ref_service_for_source (CamelSession *session, const gchar *src) { CamelService *service = NULL; /* Source strings are now CamelService UIDs. */ if (src != NULL) - service = camel_session_get_service (session, src); + service = camel_session_ref_service (session, src); /* For backward-compability, also handle CamelService URLs. */ if (service == NULL && src != NULL) { @@ -621,11 +621,11 @@ get_service_for_source (CamelSession *session, url = camel_url_new (src, NULL); if (service == NULL && url != NULL) - service = camel_session_get_service_by_url ( + service = camel_session_ref_service_by_url ( session, url, CAMEL_PROVIDER_STORE); if (service == NULL && url != NULL) - service = camel_session_get_service_by_url ( + service = camel_session_ref_service_by_url ( session, url, CAMEL_PROVIDER_TRANSPORT); if (url != NULL) @@ -655,16 +655,26 @@ header_source (struct _CamelSExp *f, } if (src) - msg_source = get_service_for_source (fms->session, src); + msg_source = ref_service_for_source (fms->session, src); - if (msg_source) { + if (msg_source != NULL) { gint ii; for (ii = 0; ii < argc && !truth; ii++) { if (argv[ii]->type == CAMEL_SEXP_RES_STRING) { - truth = msg_source == get_service_for_source (fms->session, argv[ii]->value.string); + CamelService *candidate; + + candidate = ref_service_for_source ( + fms->session, + argv[ii]->value.string); + if (candidate != NULL) { + truth = (msg_source == candidate); + g_object_unref (candidate); + } } } + + g_object_unref (msg_source); } r = camel_sexp_result_new (f, CAMEL_SEXP_RES_BOOL); diff --git a/camel/camel-sasl-popb4smtp.c b/camel/camel-sasl-popb4smtp.c index 42ceb27..f9e387f 100644 --- a/camel/camel-sasl-popb4smtp.c +++ b/camel/camel-sasl-popb4smtp.c @@ -85,7 +85,7 @@ sasl_popb4smtp_challenge_sync (CamelSasl *sasl, "popb4smtp_uid", 0, error); if (pop_uid != NULL) - service = camel_session_get_service (session, pop_uid); + service = camel_session_ref_service (session, pop_uid); else service = NULL; @@ -107,8 +107,7 @@ sasl_popb4smtp_challenge_sync (CamelSasl *sasl, CAMEL_SERVICE_ERROR_CANT_AUTHENTICATE, _("POP Before SMTP authentication attempted " "with a %s service"), type_name); - g_free (pop_uid); - return NULL; + goto exit; } if (strstr (type_name, "POP") == NULL) { @@ -117,8 +116,7 @@ sasl_popb4smtp_challenge_sync (CamelSasl *sasl, CAMEL_SERVICE_ERROR_CANT_AUTHENTICATE, _("POP Before SMTP authentication attempted " "with a %s service"), type_name); - g_free (pop_uid); - return NULL; + goto exit; } /* check if we've done it before recently in this session */ @@ -127,13 +125,13 @@ sasl_popb4smtp_challenge_sync (CamelSasl *sasl, /* need to lock around the whole thing until finished with timep */ POPB4SMTP_LOCK (lock); + timep = g_hash_table_lookup (poplast, pop_uid); if (timep) { if ((*timep + POPB4SMTP_TIMEOUT) > now) { camel_sasl_set_authenticated (sasl, TRUE); POPB4SMTP_UNLOCK (lock); - g_free (pop_uid); - return NULL; + goto exit; } } else { timep = g_malloc0 (sizeof (*timep)); @@ -151,6 +149,8 @@ sasl_popb4smtp_challenge_sync (CamelSasl *sasl, POPB4SMTP_UNLOCK (lock); +exit: + g_object_unref (service); g_free (pop_uid); return NULL; diff --git a/camel/camel-session.c b/camel/camel-session.c index ee6f9af..f713a62 100644 --- a/camel/camel-session.c +++ b/camel/camel-session.c @@ -387,7 +387,7 @@ session_add_service (CamelSession *session, CamelProvider *provider; GType service_type = G_TYPE_INVALID; - service = camel_session_get_service (session, uid); + service = camel_session_ref_service (session, uid); if (CAMEL_IS_SERVICE (service)) return service; @@ -1001,17 +1001,20 @@ camel_session_remove_service (CamelSession *session, } /** - * camel_session_get_service: + * camel_session_ref_service: * @session: a #CamelSession * @uid: a unique identifier string * * Looks up a #CamelService by its unique identifier string. The service * must have been previously added using camel_session_add_service(). * + * The returned #CamelService is referenced for thread-safety and must be + * unreferenced with g_object_unref() when finished with it. + * * Returns: a #CamelService instance, or %NULL **/ CamelService * -camel_session_get_service (CamelSession *session, +camel_session_ref_service (CamelSession *session, const gchar *uid) { CamelService *service; @@ -1023,13 +1026,16 @@ camel_session_get_service (CamelSession *session, service = g_hash_table_lookup (session->priv->services, uid); + if (service != NULL) + g_object_ref (service); + g_mutex_unlock (session->priv->services_lock); return service; } /** - * camel_session_get_service_by_url: + * camel_session_ref_service_by_url: * @session: a #CamelSession * @url: a #CamelURL * @type: a #CamelProviderType @@ -1039,14 +1045,17 @@ camel_session_get_service (CamelSession *session, * The service must have been previously added using * camel_session_add_service(). * - * Note this function is significantly slower than camel_session_get_service(). + * The returned #CamelService is referenced for thread-safety and must be + * unreferenced with g_object_unref() when finished with it. + * + * Note this function is significantly slower than camel_session_ref_service(). * * Returns: a #CamelService instance, or %NULL * * Since: 3.2 **/ CamelService * -camel_session_get_service_by_url (CamelSession *session, +camel_session_ref_service_by_url (CamelSession *session, CamelURL *url, CamelProviderType type) { @@ -1083,11 +1092,11 @@ camel_session_get_service_by_url (CamelSession *session, switch (type) { case CAMEL_PROVIDER_STORE: if (CAMEL_IS_STORE (service)) - match = service; + match = g_object_ref (service); break; case CAMEL_PROVIDER_TRANSPORT: if (CAMEL_IS_TRANSPORT (service)) - match = service; + match = g_object_ref (service); break; default: g_warn_if_reached (); diff --git a/camel/camel-session.h b/camel/camel-session.h index c1fd5cb..93b5150 100644 --- a/camel/camel-session.h +++ b/camel/camel-session.h @@ -187,9 +187,9 @@ CamelService * camel_session_add_service (CamelSession *session, GError **error); void camel_session_remove_service (CamelSession *session, CamelService *service); -CamelService * camel_session_get_service (CamelSession *session, +CamelService * camel_session_ref_service (CamelSession *session, const gchar *uid); -CamelService * camel_session_get_service_by_url +CamelService * camel_session_ref_service_by_url (CamelSession *session, CamelURL *url, CamelProviderType type); diff --git a/docs/reference/camel/camel-sections.txt b/docs/reference/camel/camel-sections.txt index 3eda039..6cc85a6 100644 --- a/docs/reference/camel/camel-sections.txt +++ b/docs/reference/camel/camel-sections.txt @@ -2295,8 +2295,8 @@ camel_session_get_user_cache_dir camel_session_get_socks_proxy camel_session_add_service camel_session_remove_service -camel_session_get_service -camel_session_get_service_by_url +camel_session_ref_service +camel_session_ref_service_by_url camel_session_list_services camel_session_remove_services camel_session_get_password -- 2.7.4