From 25d7fddd4ef5c143cf1746922ec331f3b2948954 Mon Sep 17 00:00:00 2001 From: Jon Trowbridge Date: Sat, 15 Sep 2001 06:05:29 +0000 Subject: Make "stop" do nothing but set the stopped flag, as in 2001-09-15 Jon Trowbridge * backend/ebook/e-book-listener.c (e_book_listener_stop): Make "stop" do nothing but set the stopped flag, as in e-book-view-listener.c. (e_book_listener_destroy): Clean up our queue here, rather than in e_book_listener_stop. (response_free): Added. Move the rather lengthy bit of code needed to free a EBookListenerResponse into one place. (e_book_listener_check_queue): Properly deal with the stopped flag. (e_book_listener_queue_response): If the stopped flag is set, just drop the incoming response. * backend/ebook/e-book-view-listener.c (e_book_view_listener_stop): Make "stop" do nothing but set the stopped flag. (e_book_view_listener_destroy): Move all of the clean-up that used to be in e_book_view_listener_stop here. This considerably simplifies the logic required to avoid various race conditions. (e_book_view_listener_check_queue): Properly deal with the stopped flag. (e_book_view_listener_queue_response): Drop all incoming responses if we are stopped. 2001-09-14 Jon Trowbridge * backend/pas/pas-book.c (pas_book_queue_request): Hold a reference to the book on behalf of our idle function. (pas_book_check_queue): When we are finished, drop the reference we've been holding on behalf of the idle function. * backend/pas/pas-backend-file.c (pas_backend_file_process_create_card): Hold a reference to the book_view while sending our notifications. (pas_backend_file_process_remove_card): Hold a reference to the book_view while sending our notifications. * gui/contact-editor/e-contact-quick-add.c (quick_add_unref): Remove debugging spew. * backend/ebook/e-book-util.c: Remove a lot of unused code that worked around bugs that have long since been fixed. (simple_query_disconnect): Added. Breaks out the part of simple_query_free that disconnect signals. (simple_query_free): Replace code w/ a call to simple_query_disconnect. (simple_query_sequence_complete_cb): Call simple_query_disconnect before executing the callback, so that our callbacks don't get triggered by any book changes that might occur during that callback. * backend/ebook/e-book-view-listener.c (e_book_view_listener_check_queue): Changed to be consistent with e_book_listener_check_queue. (e_book_view_listener_queue_response): Also changed to use a high-frequency timeout. * backend/ebook/e-book-listener.c (e_book_listener_check_queue): OK, I've agonized over this stupid little function, and it should now be race-free. (e_book_listener_queue_response): We process our response queue in a high-frequency timeout rather than an idle. Using an idle function leads to some tricky race conditions and bad interactions with bonobo's semi-broken attempts to take over event processing. (e_book_view_listener_stop): Manually disable our timeout and clean up. svn path=/trunk/; revision=12851 --- addressbook/backend/ebook/e-book-listener.c | 195 ++++++++++++----------- addressbook/backend/ebook/e-book-util.c | 87 +++------- addressbook/backend/ebook/e-book-view-listener.c | 96 +++++------ addressbook/backend/ebook/e-book-view.c | 6 +- addressbook/backend/ebook/e-book.c | 2 +- addressbook/backend/pas/pas-backend-file.c | 4 + addressbook/backend/pas/pas-book.c | 2 + 7 files changed, 184 insertions(+), 208 deletions(-) (limited to 'addressbook/backend') diff --git a/addressbook/backend/ebook/e-book-listener.c b/addressbook/backend/ebook/e-book-listener.c index 86b2e5cb9d..a66701847d 100644 --- a/addressbook/backend/ebook/e-book-listener.c +++ b/addressbook/backend/ebook/e-book-listener.c @@ -27,43 +27,92 @@ POA_GNOME_Evolution_Addressbook_BookListener__vepv e_book_listener_vepv; struct _EBookListenerPrivate { GList *response_queue; - gint idle_id; - gboolean idle_lock; + gint timeout_id; - guint stopped : 1; + guint timeout_lock : 1; + guint stopped : 1; }; -static gboolean -e_book_listener_check_queue (EBookListener *listener) +static void +response_free (EBookListenerResponse *resp) { - if (listener->priv->idle_lock) - return TRUE; - - listener->priv->idle_lock = TRUE; + if (resp == NULL) + return; - if (listener->priv->stopped) { - listener->priv->idle_id = 0; - goto exit_gracefully; + g_free (resp->msg); + g_free (resp->id); + + if (resp->book != CORBA_OBJECT_NIL) { + CORBA_Environment ev; + + CORBA_exception_init (&ev); + + bonobo_object_release_unref (resp->book, &ev); + + if (ev._major != CORBA_NO_EXCEPTION) { + g_warning ("e_book_listener_destroy: " + "Exception destroying book " + "in response queue!\n"); + } + + CORBA_exception_free (&ev); } - if (listener->priv->response_queue != NULL) { - gtk_signal_emit (GTK_OBJECT (listener), e_book_listener_signals [RESPONSES_QUEUED]); + if (resp->cursor != CORBA_OBJECT_NIL) { + CORBA_Environment ev; + + CORBA_exception_init (&ev); + + bonobo_object_release_unref (resp->cursor, &ev); + + if (ev._major != CORBA_NO_EXCEPTION) { + g_warning ("e_book_listener_destroy: " + "Exception destroying cursor " + "in response queue!\n"); + } + + CORBA_exception_free (&ev); } - if (listener->priv->response_queue == NULL) { - listener->priv->idle_id = 0; + if (resp->book_view != CORBA_OBJECT_NIL) { + CORBA_Environment ev; + + CORBA_exception_init (&ev); + + bonobo_object_release_unref (resp->book_view, &ev); + + if (ev._major != CORBA_NO_EXCEPTION) { + g_warning ("e_book_listener_destroy: " + "Exception destroying book_view " + "in response queue!\n"); + } + + CORBA_exception_free (&ev); } + + g_free (resp); +} - exit_gracefully: +static gboolean +e_book_listener_check_queue (EBookListener *listener) +{ + if (listener->priv->timeout_lock) + return TRUE; - listener->priv->idle_lock = FALSE; - - /* Only drop the reference when we exit for the last time. */ - if (listener->priv->idle_id == 0) { - gtk_object_unref (GTK_OBJECT (listener)); + listener->priv->timeout_lock = TRUE; + + if (listener->priv->response_queue != NULL && !listener->priv->stopped) { + gtk_signal_emit (GTK_OBJECT (listener), e_book_listener_signals [RESPONSES_QUEUED]); + } + + if (listener->priv->response_queue == NULL || listener->priv->stopped) { + listener->priv->timeout_id = 0; + listener->priv->timeout_lock = FALSE; + bonobo_object_unref (BONOBO_OBJECT (listener)); /* release the timeout's reference */ return FALSE; } + listener->priv->timeout_lock = FALSE; return TRUE; } @@ -71,16 +120,24 @@ static void e_book_listener_queue_response (EBookListener *listener, EBookListenerResponse *response) { - listener->priv->response_queue = - g_list_append (listener->priv->response_queue, - response); + if (response == NULL) + return; + + if (listener->priv->stopped) { + response_free (response); + return; + } + + listener->priv->response_queue = g_list_append (listener->priv->response_queue, response); - if (listener->priv->idle_id == 0) { + if (listener->priv->timeout_id == 0) { - /* Hold a reference to the listener until the idle function is finished. */ - gtk_object_ref (GTK_OBJECT (listener)); + /* 20 == an arbitrary small integer */ + listener->priv->timeout_id = g_timeout_add (20, (GSourceFunc) e_book_listener_check_queue, listener); - listener->priv->idle_id = g_idle_add ((GSourceFunc) e_book_listener_check_queue, listener); + /* Hold a reference on behalf of the timeout */ + bonobo_object_ref (BONOBO_OBJECT (listener)); + } } @@ -655,79 +712,31 @@ e_book_listener_init (EBookListener *listener) void e_book_listener_stop (EBookListener *listener) { + g_return_if_fail (E_IS_BOOK_LISTENER (listener)); + + listener->priv->stopped = TRUE; +} + +static void +e_book_listener_destroy (GtkObject *object) +{ + EBookListener *listener = E_BOOK_LISTENER (object); GList *l; - if (listener->priv->stopped) - return; + /* Remove our response queue handler: In theory, this can never happen since we + always hold a reference to the listener while the timeout is running. */ + if (listener->priv->timeout_id) { + g_source_remove (listener->priv->timeout_id); + } + /* Clean up anything still sitting in response_queue */ for (l = listener->priv->response_queue; l != NULL; l = l->next) { EBookListenerResponse *resp = l->data; - g_free (resp->msg); - g_free (resp->id); - - if (resp->book != CORBA_OBJECT_NIL) { - CORBA_Environment ev; - - CORBA_exception_init (&ev); - - bonobo_object_release_unref (resp->book, &ev); - - if (ev._major != CORBA_NO_EXCEPTION) { - g_warning ("e_book_listener_destroy: " - "Exception destroying book " - "in response queue!\n"); - } - - CORBA_exception_free (&ev); - } - - if (resp->cursor != CORBA_OBJECT_NIL) { - CORBA_Environment ev; - - CORBA_exception_init (&ev); - - bonobo_object_release_unref (resp->cursor, &ev); - - if (ev._major != CORBA_NO_EXCEPTION) { - g_warning ("e_book_listener_destroy: " - "Exception destroying cursor " - "in response queue!\n"); - } - - CORBA_exception_free (&ev); - } - - if (resp->book_view != CORBA_OBJECT_NIL) { - CORBA_Environment ev; - - CORBA_exception_init (&ev); - - bonobo_object_release_unref (resp->book_view, &ev); - - if (ev._major != CORBA_NO_EXCEPTION) { - g_warning ("e_book_listener_destroy: " - "Exception destroying book_view " - "in response queue!\n"); - } - - CORBA_exception_free (&ev); - } - - g_free (resp); + response_free (resp); } g_list_free (listener->priv->response_queue); - listener->priv->response_queue = NULL; - - listener->priv->stopped = TRUE; -} - -static void -e_book_listener_destroy (GtkObject *object) -{ - EBookListener *listener = E_BOOK_LISTENER (object); - e_book_listener_stop (listener); g_free (listener->priv); GTK_OBJECT_CLASS (e_book_listener_parent_class)->destroy (object); diff --git a/addressbook/backend/ebook/e-book-util.c b/addressbook/backend/ebook/e-book-util.c index c65c70dfde..3943117ce0 100644 --- a/addressbook/backend/ebook/e-book-util.c +++ b/addressbook/backend/ebook/e-book-util.c @@ -186,11 +186,6 @@ book_issue_tag (EBook *book) return tag; } -#ifdef USE_WORKAROUND -static GList *WORKAROUND_sq_queue = NULL; -static gboolean WORKAROUND_running_query = FALSE; -#endif - static SimpleQueryInfo * simple_query_new (EBook *book, const char *query, EBookSimpleQueryCallback cb, gpointer closure) { @@ -206,47 +201,37 @@ simple_query_new (EBook *book, const char *query, EBookSimpleQueryCallback cb, g /* Automatically add ourselves to the EBook's pending list. */ book_add_simple_query (book, sq); -#ifdef USE_WORKAROUND - /* Add ourselves to the queue. */ - WORKAROUND_sq_queue = g_list_append (WORKAROUND_sq_queue, sq); -#endif - return sq; } +static void +simple_query_disconnect (SimpleQueryInfo *sq) +{ + if (sq->add_tag) { + gtk_signal_disconnect (GTK_OBJECT (sq->view), sq->add_tag); + sq->add_tag = 0; + } + + if (sq->seq_complete_tag) { + gtk_signal_disconnect (GTK_OBJECT (sq->view), sq->seq_complete_tag); + sq->seq_complete_tag = 0; + } + + if (sq->view) { + gtk_object_unref (GTK_OBJECT (sq->view)); + sq->view = NULL; + } +} + static void simple_query_free (SimpleQueryInfo *sq) { /* Remove ourselves from the EBook's pending list. */ book_remove_simple_query (sq->book, sq); -#ifdef USE_WORKAROUND - Glist *i; - - /* If we are still in the queue, remove ourselves. */ - for (i = WORKAROUND_sq_queue; i != NULL; i = g_list_next (i)) { - if (i->data == sq) { - WORKAROUND_sq_queue = g_list_remove_link (WORKAROUND_sq_queue, i); - g_list_free_1 (i); - break; - } - } -#endif - g_free (sq->query); - if (sq->add_tag) - gtk_signal_disconnect (GTK_OBJECT (sq->view), sq->add_tag); - if (sq->seq_complete_tag) - gtk_signal_disconnect (GTK_OBJECT (sq->view), sq->seq_complete_tag); - -#ifdef USE_WORKAROUND - if (sq->view) - WORKAROUND_running_query = FALSE; -#endif - - if (sq->view) - gtk_object_unref (GTK_OBJECT (sq->view)); + simple_query_disconnect (sq); if (sq->book) gtk_object_unref (GTK_OBJECT (sq->book)); @@ -271,6 +256,9 @@ simple_query_sequence_complete_cb (EBookView *view, gpointer closure) { SimpleQueryInfo *sq = closure; + /* Disconnect signals, so that we don't pick up any changes to the book that occur + in our callback */ + simple_query_disconnect (sq); sq->cb (sq->book, E_BOOK_SIMPLE_QUERY_STATUS_SUCCESS, sq->cards, sq->closure); simple_query_free (sq); } @@ -299,31 +287,6 @@ simple_query_book_view_cb (EBook *book, EBookStatus status, EBookView *book_view sq); } -#ifdef USE_WORKAROUND -static gint -WORKAROUND_try_queue (gpointer foo) -{ - if (WORKAROUND_sq_queue) { - SimpleQueryInfo *sq; - GList *i; - - if (WORKAROUND_running_query) - return TRUE; - - WORKAROUND_running_query = TRUE; - sq = WORKAROUND_sq_queue->data; - - i = WORKAROUND_sq_queue; - WORKAROUND_sq_queue = g_list_remove_link (WORKAROUND_sq_queue, WORKAROUND_sq_queue); - g_list_free_1 (i); - - e_book_get_book_view (sq->book, sq->query, simple_query_book_view_cb, sq); - } - - return FALSE; -} -#endif - guint e_book_simple_query (EBook *book, const char *query, EBookSimpleQueryCallback cb, gpointer closure) { @@ -334,11 +297,7 @@ e_book_simple_query (EBook *book, const char *query, EBookSimpleQueryCallback cb g_return_val_if_fail (cb, 0); sq = simple_query_new (book, query, cb, closure); -#ifdef USE_WORKAROUND - gtk_timeout_add (50, WORKAROUND_try_queue, NULL); -#else e_book_get_book_view (book, (gchar *) query, simple_query_book_view_cb, sq); -#endif return sq->tag; } diff --git a/addressbook/backend/ebook/e-book-view-listener.c b/addressbook/backend/ebook/e-book-view-listener.c index 0b862fc61b..40e467ef93 100644 --- a/addressbook/backend/ebook/e-book-view-listener.c +++ b/addressbook/backend/ebook/e-book-view-listener.c @@ -27,44 +27,32 @@ POA_GNOME_Evolution_Addressbook_BookViewListener__vepv e_book_view_listener_vep struct _EBookViewListenerPrivate { GList *response_queue; - gint idle_id; - gboolean idle_lock; + gint timeout_id; - guint stopped : 1; + guint timeout_lock : 1; + guint stopped : 1; }; -/* Only release our listener reference when the idle is finished. */ static gboolean e_book_view_listener_check_queue (EBookViewListener *listener) { - if (listener->priv->idle_lock) + if (listener->priv->timeout_lock) return TRUE; - listener->priv->idle_lock = TRUE; + listener->priv->timeout_lock = TRUE; - if (listener->priv->stopped) { - listener->priv->idle_id = 0; - goto exit_gracefully; - } - - if (listener->priv->response_queue != NULL) { + if (listener->priv->response_queue != NULL && !listener->priv->stopped) { gtk_signal_emit (GTK_OBJECT (listener), e_book_view_listener_signals [RESPONSES_QUEUED]); } - if (listener->priv->response_queue == NULL) { - listener->priv->idle_id = 0; - } - - exit_gracefully: - - listener->priv->idle_lock = FALSE; - - /* Only drop the reference when we exit for the last time. */ - if (listener->priv->idle_id == 0) { - gtk_object_unref (GTK_OBJECT (listener)); + if (listener->priv->response_queue == NULL || listener->priv->stopped) { + listener->priv->timeout_id = 0; + listener->priv->timeout_lock = FALSE; + bonobo_object_unref (BONOBO_OBJECT (listener)); return FALSE; } + listener->priv->timeout_lock = FALSE; return TRUE; } @@ -72,14 +60,28 @@ static void e_book_view_listener_queue_response (EBookViewListener *listener, EBookViewListenerResponse *response) { + if (response == NULL) + return; + + if (listener->priv->stopped) { + /* Free response and return */ + g_free (response->id); + g_list_foreach (response->cards, (GFunc) gtk_object_unref, NULL); + g_list_free (response->cards); + g_free (response->message); + g_free (response); + return; + } + listener->priv->response_queue = g_list_append (listener->priv->response_queue, response); - if (listener->priv->idle_id == 0) { + if (listener->priv->timeout_id == 0) { - /* Hold a reference to the listener while the idle is active. */ - gtk_object_ref (GTK_OBJECT (listener)); + /* Here, 20 == an arbitrary small number */ + listener->priv->timeout_id = g_timeout_add (20, (GSourceFunc) e_book_view_listener_check_queue, listener); - listener->priv->idle_id = g_idle_add ((GSourceFunc) e_book_view_listener_check_queue, listener); + /* Hold a reference to the listener on behalf of the timeout */ + bonobo_object_ref (BONOBO_OBJECT (listener)); } } @@ -340,24 +342,31 @@ e_book_view_listener_init (EBookViewListener *listener) { listener->priv = g_new0 (EBookViewListenerPrivate, 1); listener->priv->response_queue = NULL; - listener->priv->idle_id = 0; + listener->priv->timeout_id = 0; + listener->priv->timeout_lock = FALSE; listener->priv->stopped = FALSE; } void e_book_view_listener_stop (EBookViewListener *listener) { - GList *l; - - if (listener->priv->stopped) - return; + g_return_if_fail (E_IS_BOOK_VIEW_LISTENER (listener)); + listener->priv->stopped = TRUE; +} - if (listener->priv->idle_id) { - g_source_remove(listener->priv->idle_id); - listener->priv->idle_id = 0; - bonobo_object_unref (BONOBO_OBJECT (listener)); +static void +e_book_view_listener_destroy (GtkObject *object) +{ + EBookViewListener *listener = E_BOOK_VIEW_LISTENER (object); + GList *l; + + /* Remove our response queue handler: In theory, this can never happen since we + always hold a reference to the listener while the timeout is running. */ + if (listener->priv->timeout_id) { + g_source_remove (listener->priv->timeout_id); } + /* Clear out the queue */ for (l = listener->priv->response_queue; l != NULL; l = l->next) { EBookViewListenerResponse *resp = l->data; @@ -365,24 +374,17 @@ e_book_view_listener_stop (EBookViewListener *listener) g_list_foreach(resp->cards, (GFunc) gtk_object_unref, NULL); g_list_free(resp->cards); + resp->cards = NULL; g_free (resp->message); + resp->message = NULL; g_free (resp); } g_list_free (listener->priv->response_queue); - listener->priv->response_queue = NULL; - - listener->priv->stopped = TRUE; -} - -static void -e_book_view_listener_destroy (GtkObject *object) -{ - EBookViewListener *listener = E_BOOK_VIEW_LISTENER (object); - - e_book_view_listener_stop (listener); + g_free (listener->priv); + listener->priv = NULL; GTK_OBJECT_CLASS (e_book_view_listener_parent_class)->destroy (object); } diff --git a/addressbook/backend/ebook/e-book-view.c b/addressbook/backend/ebook/e-book-view.c index 39aab8623f..e56c7e90bc 100644 --- a/addressbook/backend/ebook/e-book-view.c +++ b/addressbook/backend/ebook/e-book-view.c @@ -114,7 +114,7 @@ e_book_view_check_listener_queue (EBookViewListener *listener, EBookView *book_v EBookViewListenerResponse *resp; resp = e_book_view_listener_pop_response (listener); - + if (resp == NULL) return; @@ -170,11 +170,11 @@ e_book_view_construct (EBookView *book_view, GNOME_Evolution_Addressbook_BookVie * Create our local BookListener interface. */ book_view->priv->listener = listener; - - bonobo_object_ref(BONOBO_OBJECT(book_view->priv->listener)); book_view->priv->responses_queued_id = gtk_signal_connect (GTK_OBJECT (book_view->priv->listener), "responses_queued", e_book_view_check_listener_queue, book_view); + bonobo_object_ref(BONOBO_OBJECT(book_view->priv->listener)); + return TRUE; } diff --git a/addressbook/backend/ebook/e-book.c b/addressbook/backend/ebook/e-book.c index 856b1d92b5..7c7738835c 100644 --- a/addressbook/backend/ebook/e-book.c +++ b/addressbook/backend/ebook/e-book.c @@ -305,7 +305,7 @@ e_book_do_response_get_view (EBook *book, return; } - book_view = e_book_view_new(resp->book_view, op->listener); + book_view = e_book_view_new (resp->book_view, op->listener); if (book_view != NULL) { e_book_view_set_book (book_view, book); diff --git a/addressbook/backend/pas/pas-backend-file.c b/addressbook/backend/pas/pas-backend-file.c index f13f486d97..89da1efb9c 100644 --- a/addressbook/backend/pas/pas-backend-file.c +++ b/addressbook/backend/pas/pas-backend-file.c @@ -517,8 +517,10 @@ pas_backend_file_process_create_card (PASBackend *backend, for (iterator = e_list_get_iterator(bf->priv->book_views); e_iterator_is_valid(iterator); e_iterator_next(iterator)) { const PASBackendFileBookView *view = e_iterator_get(iterator); if (vcard_matches_search (view, vcard)) { + bonobo_object_ref (BONOBO_OBJECT (view->book_view)); pas_book_view_notify_add_1 (view->book_view, vcard); pas_book_view_notify_complete (view->book_view); + bonobo_object_unref (BONOBO_OBJECT (view->book_view)); } } gtk_object_unref(GTK_OBJECT(iterator)); @@ -584,8 +586,10 @@ pas_backend_file_process_remove_card (PASBackend *backend, for (iterator = e_list_get_iterator (bf->priv->book_views); e_iterator_is_valid(iterator); e_iterator_next(iterator)) { const PASBackendFileBookView *view = e_iterator_get(iterator); if (vcard_matches_search (view, vcard_string)) { + bonobo_object_ref (BONOBO_OBJECT (view->book_view)); pas_book_view_notify_remove (view->book_view, req->id); pas_book_view_notify_complete (view->book_view); + bonobo_object_unref (BONOBO_OBJECT (view->book_view)); } } gtk_object_unref(GTK_OBJECT(iterator)); diff --git a/addressbook/backend/pas/pas-book.c b/addressbook/backend/pas/pas-book.c index 9736648665..306d7e5f39 100644 --- a/addressbook/backend/pas/pas-book.c +++ b/addressbook/backend/pas/pas-book.c @@ -38,6 +38,7 @@ pas_book_check_queue (PASBook *book) if (book->priv->request_queue == NULL) { book->priv->idle_id = 0; + gtk_object_unref (GTK_OBJECT (book)); return FALSE; } @@ -51,6 +52,7 @@ pas_book_queue_request (PASBook *book, PASRequest *req) g_list_append (book->priv->request_queue, req); if (book->priv->idle_id == 0) { + gtk_object_ref (GTK_OBJECT (book)); book->priv->idle_id = g_idle_add ((GSourceFunc) pas_book_check_queue, book); } } -- cgit v1.2.3