Created attachment 149166 [details] [review]
Checks, if book is NULL

Review of attachment 149166 [details] [review]:

That isn't the right fix -- g_object_new() typically can be assumed to never fail.  The question is why is it trying to re-register E_TYPE_BOOK.

(In reply to comment #2)
> Review of attachment 149166 [details] [review]:
> 
> That isn't the right fix -- g_object_new() typically can be assumed to never
> fail.  The question is why is it trying to re-register E_TYPE_BOOK.

You're are right. I have set a conditional breakpoint in my .gdbinit since I run evolution 2.28 inside gdb. What should I be looking for if it happens again?

Just get a backtrace for all threads.  This is very odd, type registration is one-time.

(In reply to comment #4)
> Just get a backtrace for all threads.  This is very odd, type registration is
> one-time.

My stack trace contains backtrace for all threads. And, looking at it again, it appears as if both thread 144 and thread 1 call e_book_new(), which calls g_type_register_static(). And while thread 144 has just released its rw-lock, thread 1 gets 0 as the result of the call to g_type_register_static().

Created attachment 149306 [details] [review]
Check for type existence before registering it again in e_book_get_type

Review of attachment 149306 [details] [review]:

Oh, this is pre-dbus.  Yes, the existing code isn't threadsafe and neither is your change -- the proper fix would be to use the G_DEFINE_TYPE macro which is properly thread-safe.  I'm surprised this hasn't been seen earlier...

Created attachment 149371 [details] [review]
next try, this time with G_DEFINE_TYPE macro, which according to Ross Burton is thread-safe

Ross, it seems you are on this. I'm fine with the change, but as you were reviewing this all. I also understand your comment "pre-dbus", the actual master is using G_DEFINE_TYPE macro. Thus, from my side, feel free to commit to gnome-2-28. Thanks.

Review of attachment 149371 [details] [review]:

Sorry, this mail got lost in my inbox.

Yes, approved for the 2-28 branch (as this is already fixed in master).  Bonus points for repeating the same patch for every object in libebook and libecal. :)

(In reply to comment #10)
> Review of attachment 149371 [details] [review]:
> 
> Sorry, this mail got lost in my inbox.
> 
> Yes, approved for the 2-28 branch (as this is already fixed in master).  Bonus
> points for repeating the same patch for every object in libebook and libecal.
> :)

Hmm. Just did the same procedure in evolution-data-server/addressbook/libebook and evolution-data-server/calendar/libecal and evolution 2.28 would not load
the addressbooks, spilling out a lot of 
"invalid cast to E_IS_VCARD". 

diff --git a/addressbook/libebook/e-book-view.c b/addressbook/libebook/e-book-view.c
index 03c2327..12cbe24 100644
--- a/addressbook/libebook/e-book-view.c
+++ b/addressbook/libebook/e-book-view.c
@@ -252,6 +252,8 @@ e_book_view_stop (EBookView *book_view)
 	}
 }
 
+G_DEFINE_TYPE(EBookView, e_book_view, G_TYPE_OBJECT)
+	
 static void
 e_book_view_init (EBookView *book_view)
 {
@@ -361,30 +363,3 @@ e_book_view_class_init (EBookViewClass *klass)
 
 	object_class->dispose = e_book_view_dispose;
 }
-
-/**
- * e_book_view_get_type:
- */
-GType
-e_book_view_get_type (void)
-{
-	static GType type = 0;
-
-	if (! type) {
-		GTypeInfo info = {
-			sizeof (EBookViewClass),
-			NULL, /* base_class_init */
-			NULL, /* base_class_finalize */
-			(GClassInitFunc)  e_book_view_class_init,
-			NULL, /* class_finalize */
-			NULL, /* class_data */
-			sizeof (EBookView),
-			0,    /* n_preallocs */
-			(GInstanceInitFunc) e_book_view_init
-		};
-
-		type = g_type_register_static (G_TYPE_OBJECT, "EBookView", &info, 0);
-	}
-
-	return type;
-}
diff --git a/addressbook/libebook/e-contact.c b/addressbook/libebook/e-contact.c
index a2d6295..8bf3013 100644
--- a/addressbook/libebook/e-contact.c
+++ b/addressbook/libebook/e-contact.c
@@ -368,35 +368,14 @@ e_contact_class_init (EContactClass *klass)
 	}
 }
 
+G_DEFINE_TYPE(EContact, e_contact, G_TYPE_OBJECT)
+	
 static void
 e_contact_init (EContact *ec)
 {
 	ec->priv = g_new0 (EContactPrivate, 1);
 }
 
-GType
-e_contact_get_type (void)
-{
-	static GType contact_type = 0;
-
-	if (!contact_type) {
-		static const GTypeInfo contact_info =  {
-			sizeof (EContactClass),
-			NULL,           /* base_init */
-			NULL,           /* base_finalize */
-			(GClassInitFunc) e_contact_class_init,
-			NULL,           /* class_finalize */
-			NULL,           /* class_data */
-			sizeof (EContact),
-			0,             /* n_preallocs */
-			(GInstanceInitFunc) e_contact_init,
-		};
-
-		contact_type = g_type_register_static (E_TYPE_VCARD, "EContact", &contact_info, 0);
-	}
-
-	return contact_type;
-}
 
 static EVCardAttribute*
 e_contact_get_first_attr (EContact *contact, const gchar *attr_name)
diff --git a/addressbook/libebook/e-destination.c b/addressbook/libebook/e-destination.c
index 83b21d7..b32267b 100644
--- a/addressbook/libebook/e-destination.c
+++ b/addressbook/libebook/e-destination.c
@@ -148,6 +148,8 @@ e_destination_class_init (EDestinationClass *klass)
 	object_class->dispose = e_destination_dispose;
 }
 
+G_DEFINE_TYPE(EDestination, e_destination, G_TYPE_OBJECT)
+	
 static void
 e_destination_init (EDestination *dest)
 {
@@ -157,29 +159,6 @@ e_destination_init (EDestination *dest)
 	dest->priv->ignored = FALSE;
 }
 
-GType
-e_destination_get_type (void)
-{
-	static GType dest_type = 0;
-
-	if (!dest_type) {
-		GTypeInfo dest_info = {
-			sizeof (EDestinationClass),
-			NULL, /* base_class_init */
-			NULL, /* base_class_finalize */
-			(GClassInitFunc)  e_destination_class_init,
-			NULL, /* class_finalize */
-			NULL, /* class_data */
-			sizeof (EDestination),
-			0,    /* n_preallocs */
-			(GInstanceInitFunc) e_destination_init
-		};
-
-		dest_type = g_type_register_static (G_TYPE_OBJECT, "EDestination", &dest_info, 0);
-	}
-
-	return dest_type;
-}
 
 /**
  * e_destination_new:
diff --git a/addressbook/libebook/e-vcard.c b/addressbook/libebook/e-vcard.c
index 2398a2b..be3f7cc 100644
--- a/addressbook/libebook/e-vcard.c
+++ b/addressbook/libebook/e-vcard.c
@@ -94,36 +94,14 @@ e_vcard_class_init (EVCardClass *klass)
 	object_class->dispose = e_vcard_dispose;
 }
 
+G_DEFINE_TYPE(EVCard, e_vcard, G_TYPE_OBJECT)
+	
 static void
 e_vcard_init (EVCard *evc)
 {
 	evc->priv = g_new0 (EVCardPrivate, 1);
 }
 
-GType
-e_vcard_get_type (void)
-{
-	static GType vcard_type = 0;
-
-	if (!vcard_type) {
-		static const GTypeInfo vcard_info =  {
-			sizeof (EVCardClass),
-			NULL,           /* base_init */
-			NULL,           /* base_finalize */
-			(GClassInitFunc) e_vcard_class_init,
-			NULL,           /* class_finalize */
-			NULL,           /* class_data */
-			sizeof (EVCard),
-			0,             /* n_preallocs */
-			(GInstanceInitFunc) e_vcard_init,
-		};
-
-		vcard_type = g_type_register_static (G_TYPE_OBJECT, "EVCard", &vcard_info, 0);
-	}
-
-	return vcard_type;
-}
-
 /*  Skip newline characters and return the next character.
  *  This function takes care of folding lines, skipping
  *  newline characters if found, taking care of equal characters
diff --git a/calendar/libecal/e-cal-component.c b/calendar/libecal/e-cal-component.c
index ab3e26c..8f2c3b1 100644
--- a/calendar/libecal/e-cal-component.c
+++ b/calendar/libecal/e-cal-component.c
@@ -191,43 +191,15 @@ struct _ECalComponentAlarm {
 
 
 static void e_cal_component_class_init (ECalComponentClass *klass);
-static void e_cal_component_init (ECalComponent *comp, ECalComponentClass *klass);
+static void e_cal_component_init (ECalComponent *comp);
 static void e_cal_component_finalize (GObject *object);
 
 static GObjectClass *parent_class;
 
 
 
-/**
- * e_cal_component_get_type:
- *
- * Registers the #ECalComponent class if necessary, and returns the type ID
- * associated to it.
- *
- * Return value: The type ID of the #ECalComponent class.
- **/
-GType
-e_cal_component_get_type (void)
-{
-	static GType e_cal_component_type = 0;
-
-	if (!e_cal_component_type) {
-		static GTypeInfo info = {
-                        sizeof (ECalComponentClass),
-                        (GBaseInitFunc) NULL,
-                        (GBaseFinalizeFunc) NULL,
-                        (GClassInitFunc) e_cal_component_class_init,
-                        NULL, NULL,
-                        sizeof (ECalComponent),
-                        0,
-                        (GInstanceInitFunc) e_cal_component_init
-                };
-		e_cal_component_type = g_type_register_static (G_TYPE_OBJECT, "ECalComponent", &info, 0);
-	}
-
-	return e_cal_component_type;
-}
-
+G_DEFINE_TYPE(ECalComponent, e_cal_component, G_TYPE_OBJECT)
+	
 /* Class initialization function for the calendar component object */
 static void
 e_cal_component_class_init (ECalComponentClass *klass)
@@ -243,7 +215,7 @@ e_cal_component_class_init (ECalComponentClass *klass)
 
 /* Object initialization function for the calendar component object */
 static void
-e_cal_component_init (ECalComponent *comp, ECalComponentClass *klass)
+e_cal_component_init (ECalComponent *comp)
 {
 	ECalComponentPrivate *priv;
 
diff --git a/calendar/libecal/e-cal-view.c b/calendar/libecal/e-cal-view.c
index 23fbc96..16b3a8e 100644
--- a/calendar/libecal/e-cal-view.c
+++ b/calendar/libecal/e-cal-view.c
@@ -124,9 +124,11 @@ view_done_cb (ECalViewListener *listener, ECalendarStatus status, gpointer data)
 	g_signal_emit (G_OBJECT (view), signals[VIEW_DONE], 0, status);
 }
 
+G_DEFINE_TYPE(ECalView, e_cal_view, G_TYPE_OBJECT)
+	
 /* Object initialization function for the calendar view */
 static void
-e_cal_view_init (ECalView *view, ECalViewClass *klass)
+e_cal_view_init (ECalView *view)
 {
 	ECalViewPrivate *priv;
 
@@ -301,36 +303,6 @@ e_cal_view_class_init (ECalViewClass *klass)
 }
 
 /**
- * e_cal_view_get_type:
- *
- * Registers the #ECalView class if necessary, and returns the type ID assigned
- * to it.
- *
- * Return value: The type ID of the #ECalView class.
- **/
-GType
-e_cal_view_get_type (void)
-{
-	static GType e_cal_view_type = 0;
-
-	if (!e_cal_view_type) {
-		static GTypeInfo info = {
-                        sizeof (ECalViewClass),
-                        (GBaseInitFunc) NULL,
-                        (GBaseFinalizeFunc) NULL,
-                        (GClassInitFunc) e_cal_view_class_init,
-                        NULL, NULL,
-                        sizeof (ECalView),
-                        0,
-                        (GInstanceInitFunc) e_cal_view_init
-                };
-		e_cal_view_type = g_type_register_static (G_TYPE_OBJECT, "ECalView", &info, 0);
-	}
-
-	return e_cal_view_type;
-}
-
-/**
  * e_cal_view_new:
  * @corba_view: The CORBA object for the view.
  * @listener: An #ECalViewListener.
diff --git a/calendar/libecal/e-cal.c b/calendar/libecal/e-cal.c
index 15c6022..0cc4820 100644
--- a/calendar/libecal/e-cal.c
+++ b/calendar/libecal/e-cal.c
@@ -1053,9 +1053,11 @@ get_factories (const gchar *str_uri, GList **factories)
 	return TRUE;
 }
 
+G_DEFINE_TYPE(ECal, e_cal, G_TYPE_OBJECT)
+	
 /* Object initialization function for the calendar ecal */
 static void
-e_cal_init (ECal *ecal, ECalClass *klass)
+e_cal_init (ECal *ecal)
 {
 	ECalPrivate *priv;
 
@@ -1237,36 +1239,6 @@ e_cal_class_init (ECalClass *klass)
 	object_class->finalize = e_cal_finalize;
 }
 
-/**
- * e_cal_get_type:
- *
- * Registers the #ECal class if necessary, and returns the type ID assigned
- * to it.
- *
- * Return value: The type ID of the #ECal class.
- **/
-GType
-e_cal_get_type (void)
-{
-	static GType e_cal_type = 0;
-
-	if (!e_cal_type) {
-		static GTypeInfo info = {
-                        sizeof (ECalClass),
-                        (GBaseInitFunc) NULL,
-                        (GBaseFinalizeFunc) NULL,
-                        (GClassInitFunc) e_cal_class_init,
-                        NULL, NULL,
-                        sizeof (ECal),
-                        0,
-                        (GInstanceInitFunc) e_cal_init
-                };
-		e_cal_type = g_type_register_static (G_TYPE_OBJECT, "ECal", &info, 0);
-	}
-
-	return e_cal_type;
-}
-

(In reply to comment #10)
> Review of attachment 149371 [details] [review]:
> 
> Sorry, this mail got lost in my inbox.
> 
> Yes, approved for the 2-28 branch (as this is already fixed in master).  Bonus
> points for repeating the same patch for every object in libebook and libecal.
> :)

Okay, another try. This time, it looks okay. Error in previous patch was this line
G_DEFINE_TYPE(EContact, e_contact, G_TYPE_OBJECT)
should be 
G_DEFINE_TYPE(EContact, e_contact, E_TYPE_VCARD)

Created attachment 157202 [details] [review]
Done as proposed in comment 10

(In reply to comment #13)
> Done as proposed in comment 10

Is it a patch for 2.28? There is not planned any other update for 2.28 any more, unfortunately, and as said, the master have it in. Pity no-one noticed the patch approval in the last ~three months, otherwise it could be included in 2.28.3 at least.

I'm closing this as a duplicate of bug #603506, because there was done the same for a git master.

*** This bug has been marked as a duplicate of bug 603506 ***

Created attachment 157521 [details] [review]
Correction for the last patch

In last patch I falsely did cast to G_TYPE_OBJECT for a couple of classes. The ugly consequence was, that no calendars came up anymore!! Dooohh. Bummer!