[evolution-data-server] Expose E-Book/Cal-BackendSExp lock



commit 9f412c48cb109b20be7999d3325d82a68a781860
Author: Milan Crha <mcrha redhat com>
Date:   Thu May 23 22:28:18 2019 +0200

    Expose E-Book/Cal-BackendSExp lock
    
    The ECalBackendSExp usage could cause a deadlock when one thread
    had been starting the view, while another thread had been notifying
    existing views about created/modified/removed objects with the file
    backend, which uses its internal lock within tzcache_get_timezone()
    function. Exposing the Backend SExp lock into the public and locking
    it before starting the view helps to avoid this deadlock.
    Similar lock had been added to the EBookBackendSExp too.
    
    The backtrace of the two threads follows:
    
    Thread 2 (Thread 0x7f4735fab700 (LWP 11567)):
    0  in syscall () at /lib64/libc.so.6
    1  in g_mutex_lock_slowpath () at /lib64/libglib-2.0.so.0
    2  in e_cal_backend_sexp_match_comp (sexp=0x7f47c80682a0, comp=comp@entry=0x7f47c0018920, cache=0x2c65b00)
        at ..../src/calendar/libedata-cal/e-cal-backend-sexp.c
    3  in match_object_sexp_to_component (value=0x7f47c0018920, data=0x7f4735faab40)
        at ..../src/calendar/backends/file/e-cal-backend-file.c
    4  in g_list_foreach () at /lib64/libglib-2.0.so.0
    5  in e_cal_backend_file_start_view (backend=0x2c65b00, query=0x7f477c01a5b0)
        at ..../src/calendar/backends/file/e-cal-backend-file.c
    6  in calview_start_thread (data=0x7f477c01a5b0)
        at ..../src/calendar/libedata-cal/e-data-cal-view.c
    7  in g_thread_proxy () at /lib64/libglib-2.0.so.0
    8  in start_thread () at /lib64/libpthread.so.0
    9  in clone () at /lib64/libc.so.6
    
    Thread 1 (Thread 0x7f4820dcba80 (LWP 5054)):
    0  in __lll_lock_wait () at /lib64/libpthread.so.0
    1  in _L_lock_941 () at /lib64/libpthread.so.0
    2  in pthread_mutex_lock () at /lib64/libpthread.so.0
    3  in cal_backend_file_get_cached_timezone (cache=0x2c65b00, tzid=0x2cbc790 "America/New_York")
        at ..../src/calendar/backends/file/e-cal-backend-file.c
    4  in func_occur_in_time_range (user_data=0x2cbab10, tzid=<optimized out>)
        at ..../src/calendar/libedata-cal/e-cal-backend-sexp.c
    5  in func_occur_in_time_range (esexp=0x2c4e100, argc=<optimized out>, argv=<optimized out>, 
data=0x2cbab10)
        at ..../src/calendar/libedata-cal/e-cal-backend-sexp.c
    6  in e_sexp_term_eval (sexp=sexp@entry=0x2c4e100, t=0x2c4a400)
        at ..../src/libedataserver/e-sexp.c
    7  in e_sexp_eval (sexp=0x2c4e100)
        at ..../src/libedataserver/e-sexp.c
    8  in e_cal_backend_sexp_match_comp (sexp=0x7f47c80682a0, comp=comp@entry=0x7f47c0018f20, 
cache=<optimized out>)
        at ..../src/calendar/libedata-cal/e-cal-backend-sexp.c
    9  in e_data_cal_view_component_matches (view=view@entry=0x7f477c01a5b0, 
component=component@entry=0x7f47c0018f20)
        at ..../src/calendar/libedata-cal/e-data-cal-view.c
    10 in e_cal_backend_notify_component_created (backend=<optimized out>, component=0x7f47c0018f20)
        at ..../src/calendar/libedata-cal/e-cal-backend.c
    11 in e_cal_backend_create_objects_finish (backend=0x2c65b00, result=result@entry=0x7f47f80648a0, 
out_uids=out_uids@entry=0x7fff7bd8d200, error=error@entry=0x7fff7bd8d1f8)
        at ..../src/calendar/libedata-cal/e-cal-backend.c
    12 in data_cal_complete_create_objects_cb (source_object=0x2c65b00, result=0x7f47f80648a0, 
user_data=0x2c31d80)
        at ..../src/calendar/libedata-cal/e-data-cal.c
    13 in g_simple_async_result_complete () at /lib64/libgio-2.0.so.0
    14 in complete_in_idle_cb () at /lib64/libgio-2.0.so.0
    15 in g_idle_dispatch () at /lib64/libglib-2.0.so.0
    16 in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
    17 in g_main_context_iterate.isra.19 () at /lib64/libglib-2.0.so.0
    18 in g_main_loop_run () at /lib64/libglib-2.0.so.0
    19 in main (argc=1, argv=0x7fff7bd8d518)
        at ..../src/calendar/libedata-cal/evolution-calendar-factory-subprocess.c

 .../libedata-book/e-book-backend-sexp.c            | 41 +++++++++++++++++++++
 .../libedata-book/e-book-backend-sexp.h            |  2 +
 src/addressbook/libedata-book/e-data-book-view.c   | 10 ++++-
 src/calendar/libedata-cal/e-cal-backend-sexp.c     | 43 +++++++++++++++++++---
 src/calendar/libedata-cal/e-cal-backend-sexp.h     |  2 +
 src/calendar/libedata-cal/e-data-cal-view.c        | 10 ++++-
 6 files changed, 101 insertions(+), 7 deletions(-)
---
diff --git a/src/addressbook/libedata-book/e-book-backend-sexp.c 
b/src/addressbook/libedata-book/e-book-backend-sexp.c
index 9a6ac712d..35ac6fde0 100644
--- a/src/addressbook/libedata-book/e-book-backend-sexp.c
+++ b/src/addressbook/libedata-book/e-book-backend-sexp.c
@@ -45,6 +45,7 @@ struct _EBookBackendSExpPrivate {
        ESExp *search_sexp;
        gchar *text;
        SearchContext *search_context;
+       GRecMutex search_context_lock;
 };
 
 struct _SearchContext {
@@ -1090,6 +1091,8 @@ book_backend_sexp_finalize (GObject *object)
        g_free (priv->text);
        g_free (priv->search_context);
 
+       g_rec_mutex_clear (&priv->search_context_lock);
+
        /* Chain up to parent's finalize() method. */
        G_OBJECT_CLASS (e_book_backend_sexp_parent_class)->finalize (object);
 }
@@ -1110,6 +1113,8 @@ e_book_backend_sexp_init (EBookBackendSExp *sexp)
 {
        sexp->priv = E_BOOK_BACKEND_SEXP_GET_PRIVATE (sexp);
        sexp->priv->search_context = g_new (SearchContext, 1);
+
+       g_rec_mutex_init (&sexp->priv->search_context_lock);
 }
 
 /* 'builtin' functions */
@@ -1218,6 +1223,8 @@ e_book_backend_sexp_match_contact (EBookBackendSExp *sexp,
        g_return_val_if_fail (E_IS_BOOK_BACKEND_SEXP (sexp), FALSE);
        g_return_val_if_fail (E_IS_CONTACT (contact), FALSE);
 
+       e_book_backend_sexp_lock (sexp);
+
        sexp->priv->search_context->contact = g_object_ref (contact);
 
        r = e_sexp_eval (sexp->priv->search_sexp);
@@ -1228,6 +1235,8 @@ e_book_backend_sexp_match_contact (EBookBackendSExp *sexp,
 
        e_sexp_result_free (sexp->priv->search_sexp, r);
 
+       e_book_backend_sexp_unlock (sexp);
+
        return retval;
 }
 
@@ -1259,3 +1268,35 @@ e_book_backend_sexp_match_vcard (EBookBackendSExp *sexp,
        return retval;
 }
 
+/**
+ * e_book_backend_sexp_lock:
+ * @sexp: an #EBookBackendSExp
+ *
+ * Locks the @sexp. Other threads cannot use it until
+ * it's unlocked with e_book_backend_sexp_unlock().
+ *
+ * Since: 3.34
+ **/
+void
+e_book_backend_sexp_lock (EBookBackendSExp *sexp)
+{
+       g_return_if_fail (E_IS_BOOK_BACKEND_SEXP (sexp));
+
+       g_rec_mutex_lock (&sexp->priv->search_context_lock);
+}
+
+/**
+ * e_book_backend_sexp_unlock:
+ * @sexp: an #EBookBackendSExp
+ *
+ * Unlocks the @sexp, previously locked by e_book_backend_sexp_lock().
+ *
+ * Since: 3.34
+ **/
+void
+e_book_backend_sexp_unlock (EBookBackendSExp *sexp)
+{
+       g_return_if_fail (E_IS_BOOK_BACKEND_SEXP (sexp));
+
+       g_rec_mutex_unlock (&sexp->priv->search_context_lock);
+}
diff --git a/src/addressbook/libedata-book/e-book-backend-sexp.h 
b/src/addressbook/libedata-book/e-book-backend-sexp.h
index 42a9d6fb2..1499daa32 100644
--- a/src/addressbook/libedata-book/e-book-backend-sexp.h
+++ b/src/addressbook/libedata-book/e-book-backend-sexp.h
@@ -84,6 +84,8 @@ gboolean      e_book_backend_sexp_match_vcard (EBookBackendSExp *sexp,
 gboolean       e_book_backend_sexp_match_contact
                                                (EBookBackendSExp *sexp,
                                                 EContact *contact);
+void           e_book_backend_sexp_lock        (EBookBackendSExp *sexp);
+void           e_book_backend_sexp_unlock      (EBookBackendSExp *sexp);
 
 G_END_DECLS
 
diff --git a/src/addressbook/libedata-book/e-data-book-view.c 
b/src/addressbook/libedata-book/e-data-book-view.c
index ddaa5bdb5..ee259f865 100644
--- a/src/addressbook/libedata-book/e-data-book-view.c
+++ b/src/addressbook/libedata-book/e-data-book-view.c
@@ -223,8 +223,16 @@ bookview_start_thread (gpointer data)
 {
        EDataBookView *view = data;
 
-       if (view->priv->running)
+       if (view->priv->running) {
+               /* To avoid race condition when one thread is starting the view, while
+                  another thread wants to notify about created/modified/removed objects. */
+               e_book_backend_sexp_lock (view->priv->sexp);
+
                e_book_backend_start_view (view->priv->backend, view);
+
+               e_book_backend_sexp_unlock (view->priv->sexp);
+       }
+
        g_object_unref (view);
 
        return NULL;
diff --git a/src/calendar/libedata-cal/e-cal-backend-sexp.c b/src/calendar/libedata-cal/e-cal-backend-sexp.c
index 4468e3d69..5ce1e11c6 100644
--- a/src/calendar/libedata-cal/e-cal-backend-sexp.c
+++ b/src/calendar/libedata-cal/e-cal-backend-sexp.c
@@ -43,7 +43,7 @@ struct _ECalBackendSExpPrivate {
        ESExp *search_sexp;
        gchar *text;
        SearchContext *search_context;
-       GMutex search_context_lock;
+       GRecMutex search_context_lock;
 };
 
 struct _SearchContext {
@@ -1153,7 +1153,7 @@ cal_backend_sexp_finalize (GObject *object)
        g_object_unref (priv->search_sexp);
        g_free (priv->text);
        g_free (priv->search_context);
-       g_mutex_clear (&priv->search_context_lock);
+       g_rec_mutex_clear (&priv->search_context_lock);
 
        /* Chain up to parent's finalize() method. */
        G_OBJECT_CLASS (e_cal_backend_sexp_parent_class)->finalize (object);
@@ -1176,7 +1176,7 @@ e_cal_backend_sexp_init (ECalBackendSExp *sexp)
        sexp->priv = E_CAL_BACKEND_SEXP_GET_PRIVATE (sexp);
        sexp->priv->search_context = g_new (SearchContext, 1);
 
-       g_mutex_init (&sexp->priv->search_context_lock);
+       g_rec_mutex_init (&sexp->priv->search_context_lock);
 }
 
 /* 'builtin' functions */
@@ -1303,7 +1303,7 @@ e_cal_backend_sexp_match_comp (ECalBackendSExp *sexp,
        g_return_val_if_fail (E_IS_CAL_COMPONENT (comp), FALSE);
        g_return_val_if_fail (E_IS_TIMEZONE_CACHE (cache), FALSE);
 
-       g_mutex_lock (&sexp->priv->search_context_lock);
+       e_cal_backend_sexp_lock (sexp);
 
        sexp->priv->search_context->comp = g_object_ref (comp);
        sexp->priv->search_context->cache = g_object_ref (cache);
@@ -1317,7 +1317,7 @@ e_cal_backend_sexp_match_comp (ECalBackendSExp *sexp,
 
        e_sexp_result_free (sexp->priv->search_sexp, r);
 
-       g_mutex_unlock (&sexp->priv->search_context_lock);
+       e_cal_backend_sexp_unlock (sexp);
 
        return retval;
 }
@@ -1355,6 +1355,39 @@ e_cal_backend_sexp_match_object (ECalBackendSExp *sexp,
        return retval;
 }
 
+/**
+ * e_cal_backend_sexp_lock:
+ * @sexp: an #ECalBackendSExp
+ *
+ * Locks the @sexp. Other threads cannot use it until
+ * it's unlocked with e_cal_backend_sexp_unlock().
+ *
+ * Since: 3.34
+ **/
+void
+e_cal_backend_sexp_lock (ECalBackendSExp *sexp)
+{
+       g_return_if_fail (E_IS_CAL_BACKEND_SEXP (sexp));
+
+       g_rec_mutex_lock (&sexp->priv->search_context_lock);
+}
+
+/**
+ * e_cal_backend_sexp_unlock:
+ * @sexp: an #ECalBackendSExp
+ *
+ * Unlocks the @sexp, previously locked by e_cal_backend_sexp_lock().
+ *
+ * Since: 3.34
+ **/
+void
+e_cal_backend_sexp_unlock (ECalBackendSExp *sexp)
+{
+       g_return_if_fail (E_IS_CAL_BACKEND_SEXP (sexp));
+
+       g_rec_mutex_unlock (&sexp->priv->search_context_lock);
+}
+
 /**
  * e_cal_backend_sexp_func_time_now: (skip)
  * @esexp: An #ESExp object.
diff --git a/src/calendar/libedata-cal/e-cal-backend-sexp.h b/src/calendar/libedata-cal/e-cal-backend-sexp.h
index 5f5923a9a..d369df46f 100644
--- a/src/calendar/libedata-cal/e-cal-backend-sexp.h
+++ b/src/calendar/libedata-cal/e-cal-backend-sexp.h
@@ -84,6 +84,8 @@ gboolean      e_cal_backend_sexp_match_object (ECalBackendSExp *sexp,
 gboolean       e_cal_backend_sexp_match_comp   (ECalBackendSExp *sexp,
                                                 ECalComponent *comp,
                                                 ETimezoneCache *cache);
+void           e_cal_backend_sexp_lock         (ECalBackendSExp *sexp);
+void           e_cal_backend_sexp_unlock       (ECalBackendSExp *sexp);
 
 /* Default implementations of time functions for use by subclasses */
 
diff --git a/src/calendar/libedata-cal/e-data-cal-view.c b/src/calendar/libedata-cal/e-data-cal-view.c
index f6f08c94a..1c8edb77f 100644
--- a/src/calendar/libedata-cal/e-data-cal-view.c
+++ b/src/calendar/libedata-cal/e-data-cal-view.c
@@ -160,8 +160,16 @@ calview_start_thread (gpointer data)
 {
        EDataCalView *view = data;
 
-       if (view->priv->started && !view->priv->stopped)
+       if (view->priv->started && !view->priv->stopped) {
+               /* To avoid race condition when one thread is starting the view, while
+                  another thread wants to notify about created/modified/removed objects. */
+               e_cal_backend_sexp_lock (view->priv->sexp);
+
                e_cal_backend_start_view (view->priv->backend, view);
+
+               e_cal_backend_sexp_unlock (view->priv->sexp);
+       }
+
        g_object_unref (view);
 
        return NULL;


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]