GNOME.org
 

[evolution-patches] e-d-s patch for various addressbook issues.

This plugs a few leaks in libebook, and adds code to the
e_book_view_listener to start and stop handling of requests.

The big change, though, is the change in poa policy for EDataBookView's
from PER_REQUEST to PER_OBJECT.  This means that backends that want to
search synchronously need to start their own thread.  Spawning another
thread, though, is *much* simpler than all the race prevention crap we
had to deal with before.  Check out the file backend after this change.
start_book_view and stop_book_view are much, much simpler.  The ldap
backend is even easier.

Chris

? backends/groupwise/Makefile
? backends/groupwise/Makefile.in
? backends/groupwise/create-account
Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution-data-server/addressbook/ChangeLog,v
retrieving revision 1.127
diff -u -r1.127 ChangeLog
--- ChangeLog	15 Apr 2004 08:38:59 -0000	1.127
+++ ChangeLog	15 Apr 2004 16:56:13 -0000
@@ -1,3 +1,70 @@
+2004-04-15  Chris Toshok  <toshok ximian com>
+
+	* backends/ldap/e-book-backend-ldap.c
+	(e_book_backend_ldap_start_book_view): remove the race prevention
+	stuff from here, since we set up everything before returning and
+	we know that stop_book_view won't be called before we return.
+	(e_book_backend_ldap_stop_book_view): same.
+
+	* backends/file/e-book-backend-file.c (FileBackendSearchClosure):
+	add slots for a GCond and GThread, as well as for our backend
+	here.
+	(closure_destroy): destroy the cvar.
+	(init_closure): init the cvar, thread, and backend slots.
+	(book_view_thread): new function - this contains the majority of
+	the code in the old ::start_book_view, except it lacks any of the
+	race prevention code, which it no longer needs.  We use the cvar
+	to keep the ::start_book_view thread waiting until the view is
+	actually set up and is about to enter the loop, so we know that
+	the search closure has been initialized.
+	(e_book_backend_file_start_book_view): start up the
+	book_view_thread with the closure's mutex locked so it'll pause
+	until we wait on the condition variable.  by the time we return
+	from this method we know the book view thread has started and is
+	at the point where the loop starts.
+	(e_book_backend_file_stop_book_view): set the stopped closure
+	field to TRUE and join on the thread.  remove all the race
+	prevention code from here too.  yay.
+
+	* libedata-book/e-data-book-view.c (e_data_book_view_new): switch
+	from PER_REQUEST to PER_OBJECT.  This means that all requests are
+	serialized per object, but come in on a thread specific to this
+	object.
+	(e_data_book_view_dispose): destroy the id's hash table.
+
+	* libebook/e-book-view.c (e_book_view_start): start our
+	EBookViewListener.
+	(e_book_view_stop): stop the listener.
+
+	* libebook/e-book-view-listener.h: add prototypes for
+	e_book_view_listener_start and _stop.
+
+	* libebook/e-book-view-listener.c
+	(impl_BookViewListener_notify_contacts_added): add the thread id
+	to the spew.
+	(impl_BookViewListener_notify_contacts_removed): same.
+	(impl_BookViewListener_notify_contacts_changed): same.
+	(impl_BookViewListener_notify_sequence_complete): same.
+	(impl_BookViewListener_notify_progress): same.
+	(e_book_view_listener_init): we start off in a stopped state.
+	(e_book_view_listener_start): new function - set stopped to FALSE.
+	The next message queued will create the idle handler.
+	(e_book_view_listener_stop): remove the idle handler and set
+	stopped to TRUE - this will cause all _queue_message calls to drop
+	the messages.
+
+	* libebook/e-contact.c (struct _EContactPrivate): add "int
+	padding", just so that g_new (EContactPrivate, 1) doesn't return
+	NULL.
+	(e_contact_dispose): similar change to e-vcard's dispose method.
+	don't return early if ->priv == NULL, as we need to chain up to
+	the parent's dispoes method.  this and/or the struct
+	_EContactPrivate change plug a memory leak as well that was
+	causing the EVCardPrivate to go unfreed.
+
+	* libebook/e-vcard.c (e_vcard_dispose): don't return if evc->priv
+	is NULL - we still need to chain up the parent's dispose method.
+
 2004-04-15  Sivaiah Nallagatla <snallagatla novell com>
 
 	* backends/groupwise/e-book-backend-groupwise.c 
Index: backends/file/e-book-backend-file.c
===================================================================
RCS file: /cvs/gnome/evolution-data-server/addressbook/backends/file/e-book-backend-file.c,v
retrieving revision 1.15
diff -u -r1.15 e-book-backend-file.c
--- backends/file/e-book-backend-file.c	14 Apr 2004 18:28:29 -0000	1.15
+++ backends/file/e-book-backend-file.c	15 Apr 2004 16:56:14 -0000
@@ -32,7 +32,7 @@
 #include <libedata-book/e-data-book-view.h>
 #include "e-book-backend-file.h"
 
-#define d(x)
+#define d(x) x
 
 #define CHANGES_DB_SUFFIX ".changes.db"
 
@@ -364,7 +364,10 @@
 }
 
 typedef struct {
+	EBookBackendFile *bf;
 	GMutex *mutex;
+	GCond *cond;
+	GThread *thread;
 	gboolean stopped;
 } FileBackendSearchClosure;
 
@@ -373,15 +376,19 @@
 {
 	d(printf ("destroying search closure\n"));
 	g_mutex_free (closure->mutex);
+	g_cond_free (closure->cond);
 	g_free (closure);
 }
 
 static FileBackendSearchClosure*
-init_closure (EDataBookView *book_view)
+init_closure (EDataBookView *book_view, EBookBackendFile *bf)
 {
 	FileBackendSearchClosure *closure = g_new (FileBackendSearchClosure, 1);
 
-	closure->mutex = g_mutex_new();
+	closure->bf = bf;
+	closure->mutex = g_mutex_new ();
+	closure->cond = g_cond_new ();
+	closure->thread = NULL;
 	closure->stopped = FALSE;
 
 	g_object_set_data_full (G_OBJECT (book_view), "EBookBackendFile.BookView::closure",
@@ -396,12 +403,12 @@
 	return g_object_get_data (G_OBJECT (book_view), "EBookBackendFile.BookView::closure");
 }
 
-static void
-e_book_backend_file_start_book_view (EBookBackend  *backend,
-				     EDataBookView *book_view)
+static gpointer
+book_view_thread (gpointer data)
 {
-	FileBackendSearchClosure *closure;
-	EBookBackendFile *bf = E_BOOK_BACKEND_FILE (backend);
+	EDataBookView *book_view = data;
+	FileBackendSearchClosure *closure = get_closure (book_view);
+	EBookBackendFile *bf = closure->bf;
 	const char *query;
 	DB  *db;
 	DBT id_dbt, vcard_dbt;
@@ -410,30 +417,9 @@
 
 	d(printf ("starting initial population of book view\n"));
 
-	g_mutex_lock (e_data_book_view_get_mutex (book_view));
-
-	closure = get_closure (book_view);
-	if (closure) {
-		/* the only way op can be set here is if we raced with
-		   stop_book_view and lost.  make sure by checking
-		   op->stopped. */
-		if (!closure->stopped) {
-			g_warning ("lost race with stop_book_view, but op->stopped != TRUE");
-		}
-		g_object_set_data (G_OBJECT (book_view), "EBookBackendFile.BookView::closure", NULL);
-		g_mutex_unlock (e_data_book_view_get_mutex (book_view));
-		return;
-	}
-	else {
-		closure = init_closure (book_view);
-		closure->stopped = FALSE;
-	}
-
 	/* ref the book view because it'll be removed and unrefed
 	   when/if it's stopped */
 	bonobo_object_ref (book_view);
-
-	g_mutex_unlock (e_data_book_view_get_mutex (book_view));
 	
 	db = bf->priv->file_db;
 	query = e_data_book_view_get_card_query (book_view);
@@ -443,6 +429,11 @@
 	else
 		e_data_book_view_notify_status_message (book_view, _("Searching..."));
 
+	d(printf ("signalling parent thread\n"));
+	g_mutex_lock (closure->mutex);
+	g_cond_signal (closure->cond);
+	g_mutex_unlock (closure->mutex);
+
 	if (e_book_backend_summary_is_summary_query (bf->priv->summary, query)) {
 		/* do a summary query */
 		GPtrArray *ids = e_book_backend_summary_search (bf->priv->summary, e_data_book_view_get_card_query (book_view));
@@ -516,32 +507,45 @@
 	/* unref the */
 	bonobo_object_unref (book_view);
 
-	d(printf ("finished initial population of book view\n"));
+	d(printf ("finished population of book view\n"));
+
+	return NULL;
 }
 
 static void
-e_book_backend_file_stop_book_view (EBookBackend  *backend,
-				    EDataBookView *book_view)
+e_book_backend_file_start_book_view (EBookBackend  *backend,
+				     EDataBookView *book_view)
 {
-	FileBackendSearchClosure *closure;
+	FileBackendSearchClosure *closure = init_closure (book_view, E_BOOK_BACKEND_FILE (backend));
 
-	g_mutex_lock (e_data_book_view_get_mutex (book_view));
+	g_mutex_lock (closure->mutex);
 
-	closure = get_closure (book_view);
+	d(printf ("starting book view thread\n"));
+	closure->thread = g_thread_create (book_view_thread, book_view, TRUE, NULL);
 
-	if (closure) {
-		d(printf ("stopping running query!\n"));
-		g_mutex_lock (closure->mutex);
-		closure->stopped = TRUE;
-		g_mutex_unlock (closure->mutex);
-	}
-	else {
-		d(printf ("either the query is already finished or hasn't started yet (we won the race)\n"));
-		closure = init_closure (book_view);
-		closure->stopped = TRUE;
-	}
+	g_cond_wait (closure->cond, closure->mutex);
+
+	/* at this point we know the book view thread is actually running */
+	g_mutex_unlock (closure->mutex);
+	d(printf ("returning from start_book_view\n"));
+}
+
+static void
+e_book_backend_file_stop_book_view (EBookBackend  *backend,
+				    EDataBookView *book_view)
+{
+	FileBackendSearchClosure *closure = get_closure (book_view);
+	gboolean need_join = FALSE;
+
+	d(printf ("stopping query\n"));
+	g_mutex_lock (closure->mutex);
+	if (!closure->stopped)
+		need_join = TRUE;
+	closure->stopped = TRUE;
+	g_mutex_unlock (closure->mutex);
 
-	g_mutex_unlock (e_data_book_view_get_mutex (book_view));
+	if (need_join)
+		g_thread_join (closure->thread);
 }
 
 typedef struct {
Index: backends/ldap/e-book-backend-ldap.c
===================================================================
RCS file: /cvs/gnome/evolution-data-server/addressbook/backends/ldap/e-book-backend-ldap.c,v
retrieving revision 1.15
diff -u -r1.15 e-book-backend-ldap.c
--- backends/ldap/e-book-backend-ldap.c	14 Apr 2004 18:32:02 -0000	1.15
+++ backends/ldap/e-book-backend-ldap.c	15 Apr 2004 16:56:15 -0000
@@ -2986,30 +2986,10 @@
 				     EDataBookView *view)
 {
 	EBookBackendLDAP *bl = E_BOOK_BACKEND_LDAP (backend);
-	LDAPSearchOp *op;
 
 	d(printf ("start_book_view (%p)\n", view));
 
-	g_mutex_lock (e_data_book_view_get_mutex (view));
-
-	op = g_object_get_data (G_OBJECT (view), "EBookBackendLDAP.BookView::search_op");
-
-	if (op) {
-		/* the only way op can be set here is if we raced with
-		   stop_book_view and lost.  make sure by checking
-		   op->aborted. */
-		if (!op->aborted) {
-			g_warning ("lost race with stop_book_view, but op->aborted != TRUE");
-		}
-
-		g_free (op);
-		g_object_set_data (G_OBJECT (view), "EBookBackendLDAP.BookView::search_op", NULL);
-	}
-	else {
-		e_book_backend_ldap_search (bl, NULL /* XXX ugh */, view);
-	}
-
-	g_mutex_unlock (e_data_book_view_get_mutex (view));
+	e_book_backend_ldap_search (bl, NULL /* XXX ugh */, view);
 }
 
 static void
@@ -3020,19 +3000,9 @@
 
 	d(printf ("stop_book_view (%p)\n", view));
 
-	g_mutex_lock (e_data_book_view_get_mutex (view));
-
 	op = g_object_get_data (G_OBJECT (view), "EBookBackendLDAP.BookView::search_op");
-	if (op) {
+	if (op)
 		ldap_op_finished ((LDAPOp*)op);
-	}
-	else {
-		op = g_new0 (LDAPSearchOp, 1);
-		op->aborted = TRUE;
-		g_object_set_data (G_OBJECT (view), "EBookBackendLDAP.BookView::search_op", op);
-	}
-
-	g_mutex_unlock (e_data_book_view_get_mutex (view));
 }
 
 static void
Index: libebook/e-book-listener.c
===================================================================
RCS file: /cvs/gnome/evolution-data-server/addressbook/libebook/e-book-listener.c,v
retrieving revision 1.8
diff -u -r1.8 e-book-listener.c
--- libebook/e-book-listener.c	24 Feb 2004 23:44:39 -0000	1.8
+++ libebook/e-book-listener.c	15 Apr 2004 16:56:16 -0000
@@ -173,7 +173,7 @@
 
 	g_signal_emit (listener, e_book_listener_signals [RESPONSE], 0, &response);
 
-	bonobo_object_release_unref (book_view, ev);
+	bonobo_object_release_unref (response.book_view, ev);
 }
 
 static void
Index: libebook/e-book-view-listener.c
===================================================================
RCS file: /cvs/gnome/evolution-data-server/addressbook/libebook/e-book-view-listener.c,v
retrieving revision 1.6
diff -u -r1.6 e-book-view-listener.c
--- libebook/e-book-view-listener.c	20 Mar 2004 02:22:16 -0000	1.6
+++ libebook/e-book-view-listener.c	15 Apr 2004 16:56:16 -0000
@@ -17,7 +17,7 @@
 #include "e-contact.h"
 #include "e-book-marshal.h"
 
-#define d(x)
+#define d(x) x
 
 static EBookViewStatus e_book_view_listener_convert_status (GNOME_Evolution_Addressbook_CallStatus status);
 
@@ -197,7 +197,7 @@
 {
 	EBookViewListener *listener = E_BOOK_VIEW_LISTENER (bonobo_object (servant));
 
-	d(printf ("impl_BookViewListener_notify_contacts_added\n"));
+	d(printf ("%p: impl_BookViewListener_notify_contacts_added (%p)\n", pthread_self(), listener));
 
 	e_book_view_listener_queue_sequence_event (
 		listener, ContactsAddedEvent, vcards);
@@ -210,7 +210,7 @@
 {
 	EBookViewListener *listener = E_BOOK_VIEW_LISTENER (bonobo_object (servant));
 
-	d(printf ("impl_BookViewListener_notify_contacts_removed\n"));
+	d(printf ("%p: impl_BookViewListener_notify_contacts_removed (%p)\n", pthread_self(), listener));
 
 	e_book_view_listener_queue_idlist_event (listener, ContactsRemovedEvent, ids);
 }
@@ -222,7 +222,7 @@
 {
 	EBookViewListener *listener = E_BOOK_VIEW_LISTENER (bonobo_object (servant));
 
-	d(printf ("impl_BookViewListener_notify_contacts_changed\n"));
+	d(printf ("%p: impl_BookViewListener_notify_contacts_changed (%p)\n", pthread_self(), listener));
 
 	e_book_view_listener_queue_sequence_event (
 		listener, ContactsModifiedEvent, vcards);
@@ -235,7 +235,7 @@
 {
 	EBookViewListener *listener = E_BOOK_VIEW_LISTENER (bonobo_object (servant));
 
-	d(printf ("impl_BookViewListener_notify_sequence_complete\n"));
+	d(printf ("%p: impl_BookViewListener_notify_sequence_complete (%p)\n", pthread_self(), listener));
 
 	e_book_view_listener_queue_status_event (listener, SequenceCompleteEvent,
 						 e_book_view_listener_convert_status (status));
@@ -249,7 +249,7 @@
 {
 	EBookViewListener *listener = E_BOOK_VIEW_LISTENER (bonobo_object (servant));
 
-	d(printf ("impl_BookViewListener_notify_progress\n"));
+	d(printf ("%p: impl_BookViewListener_notify_progress (%p,`%s')\n", pthread_self(), listener, message));
 
 	e_book_view_listener_queue_message_event (listener, StatusMessageEvent, message);
 }
@@ -287,7 +287,7 @@
 	EBookViewListener *listener;
 
 	listener = g_object_new (E_TYPE_BOOK_VIEW_LISTENER,
-				 "poa", bonobo_poa_get_threaded (ORBIT_THREAD_HINT_PER_REQUEST, NULL),
+				 "poa", bonobo_poa_get_threaded (ORBIT_THREAD_HINT_PER_OBJECT, NULL),
 				 NULL);
 
 	listener->priv->queue = g_async_queue_new();
@@ -301,14 +301,28 @@
 e_book_view_listener_init (EBookViewListener *listener)
 {
 	listener->priv                 = g_new0 (EBookViewListenerPrivate, 1);
-	listener->priv->stopped        = FALSE;
+	listener->priv->stopped        = TRUE;
+}
+
+void
+e_book_view_listener_start (EBookViewListener *listener)
+{
+	g_return_if_fail (E_IS_BOOK_VIEW_LISTENER (listener));
+	d(printf ("%p: e_book_view_listener_start (%p)\n", pthread_self(), listener));
+	listener->priv->stopped = FALSE;
 }
 
 void
 e_book_view_listener_stop (EBookViewListener *listener)
 {
 	g_return_if_fail (E_IS_BOOK_VIEW_LISTENER (listener));
+	d(printf ("%p: e_book_view_listener_stop (%p)\n", pthread_self(), listener));
 	listener->priv->stopped = TRUE;
+
+	if (listener->priv->idle_id != -1) {
+		g_source_remove (listener->priv->idle_id);
+		listener->priv->idle_id = -1;
+	}
 }
 
 static void
Index: libebook/e-book-view-listener.h
===================================================================
RCS file: /cvs/gnome/evolution-data-server/addressbook/libebook/e-book-view-listener.h,v
retrieving revision 1.3
diff -u -r1.3 e-book-view-listener.h
--- libebook/e-book-view-listener.h	23 Dec 2003 22:36:53 -0000	1.3
+++ libebook/e-book-view-listener.h	15 Apr 2004 16:56:16 -0000
@@ -84,6 +84,7 @@
 
 EBookViewListener         *e_book_view_listener_new            (void);
 GType                      e_book_view_listener_get_type       (void);
+void                       e_book_view_listener_start          (EBookViewListener *listener);
 void                       e_book_view_listener_stop           (EBookViewListener *listener);
 
 G_END_DECLS
Index: libebook/e-book-view.c
===================================================================
RCS file: /cvs/gnome/evolution-data-server/addressbook/libebook/e-book-view.c,v
retrieving revision 1.4
diff -u -r1.4 e-book-view.c
--- libebook/e-book-view.c	11 Feb 2004 17:34:20 -0000	1.4
+++ libebook/e-book-view.c	15 Apr 2004 16:56:16 -0000
@@ -179,6 +179,8 @@
 
 	CORBA_exception_init (&ev);
 
+	e_book_view_listener_start (book_view->priv->listener);
+
 	GNOME_Evolution_Addressbook_BookView_start (book_view->priv->corba_book_view, &ev);
 
 	if (ev._major != CORBA_NO_EXCEPTION) {
@@ -194,6 +196,8 @@
 	g_return_if_fail (book_view && E_IS_BOOK_VIEW (book_view));
 
 	CORBA_exception_init (&ev);
+
+	e_book_view_listener_stop (book_view->priv->listener);
 
 	GNOME_Evolution_Addressbook_BookView_stop (book_view->priv->corba_book_view, &ev);
 
Index: libebook/e-contact.c
===================================================================
RCS file: /cvs/gnome/evolution-data-server/addressbook/libebook/e-contact.c,v
retrieving revision 1.30
diff -u -r1.30 e-contact.c
--- libebook/e-contact.c	14 Apr 2004 18:33:30 -0000	1.30
+++ libebook/e-contact.c	15 Apr 2004 16:56:17 -0000
@@ -33,6 +33,7 @@
 #define d(x)
 
 struct _EContactPrivate {
+	int padding;
 };
 
 #define E_CONTACT_FIELD_TYPE_STRING       0x00000001   /* used for simple single valued attributes */
@@ -251,13 +252,13 @@
 {
 	EContact *ec = E_CONTACT (object);
 
-	if (!ec->priv)
-		return;
+	if (ec->priv) {
 
-	/* XXX free instance specific stuff */
+		/* XXX free instance specific stuff */
 
-	g_free (ec->priv);
-	ec->priv = NULL;
+		g_free (ec->priv);
+		ec->priv = NULL;
+	}
 
 	if (G_OBJECT_CLASS (parent_class)->dispose)
 		G_OBJECT_CLASS (parent_class)->dispose (object);
Index: libebook/e-vcard.c
===================================================================
RCS file: /cvs/gnome/evolution-data-server/addressbook/libebook/e-vcard.c,v
retrieving revision 1.12
diff -u -r1.12 e-vcard.c
--- libebook/e-vcard.c	2 Apr 2004 07:23:20 -0000	1.12
+++ libebook/e-vcard.c	15 Apr 2004 16:56:17 -0000
@@ -68,14 +68,14 @@
 {
 	EVCard *evc = E_VCARD (object);
 
-	if (!evc->priv)
-		return;
+	if (evc->priv) {
 
-	g_list_foreach (evc->priv->attributes, (GFunc)e_vcard_attribute_free, NULL);
-	g_list_free (evc->priv->attributes);
+		g_list_foreach (evc->priv->attributes, (GFunc)e_vcard_attribute_free, NULL);
+		g_list_free (evc->priv->attributes);
 
-	g_free (evc->priv);
-	evc->priv = NULL;
+		g_free (evc->priv);
+		evc->priv = NULL;
+	}
 
 	if (G_OBJECT_CLASS (parent_class)->dispose)
 		G_OBJECT_CLASS (parent_class)->dispose (object);
Index: libedata-book/e-data-book-view.c
===================================================================
RCS file: /cvs/gnome/evolution-data-server/addressbook/libedata-book/e-data-book-view.c,v
retrieving revision 1.7
diff -u -r1.7 e-data-book-view.c
--- libedata-book/e-data-book-view.c	31 Mar 2004 17:18:34 -0000	1.7
+++ libedata-book/e-data-book-view.c	15 Apr 2004 16:56:18 -0000
@@ -454,7 +454,7 @@
 	EDataBookView *book_view;
 
 	book_view = g_object_new (E_TYPE_DATA_BOOK_VIEW,
-				  "poa", bonobo_poa_get_threaded (ORBIT_THREAD_HINT_PER_REQUEST, NULL),
+				  "poa", bonobo_poa_get_threaded (ORBIT_THREAD_HINT_PER_OBJECT, NULL),
 				  NULL);
 	
 	e_data_book_view_construct (book_view, backend, listener, card_query, card_sexp, max_results);
@@ -482,10 +482,10 @@
 		g_object_unref (book_view->priv->card_sexp);
 
 		g_mutex_free (book_view->priv->pending_mutex);
-		book_view->priv->pending_mutex = NULL;
 
 		g_mutex_free (book_view->priv->mutex);
-		book_view->priv->mutex = NULL;
+
+		g_hash_table_destroy (book_view->priv->ids);
 
 		g_free (book_view->priv);
 		book_view->priv = NULL;
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]