Does e-addressbook-factory crash for you before evolution hangs ?

Doesn't look like it. If i run e-addressbook-factory before evo in a shell it doesn't die.

I'm debugging this, and it seem there is some issue in the gdbus stuff. It properly send the getContacts message and gets the reply, then it starts waiting for the done signal. We get a done signal, but the dbus method is called "get_contacts_done", and the expected_interfaces for the signals say "getContacts_done", so we don't match this signal correctly so its not emitted in the client.

I don't know where the error is, on the server side, emitting the signal with the get_contacts_done name, or on the client side expecting a getContacts_done name.

Created attachment 188738 [details] [review]
Make done signals have the right name

done signals get sent over dbus as e.g. get_contact_done, and are
then emitted locally with this signal name. However, we're currently
registering the GDBusSignalInfo->name based on the method name, i.e.
getContact_done, so the received done signal will be ignored.

This changes the dbus macros so that we get the right signal names.

Created attachment 188739 [details] [review]
Make done signals have the right name

done signals get sent over dbus as e.g. get_contact_done, and are
then emitted locally with this signal name. However, we're currently
registering the GDBusSignalInfo->name based on the method name, i.e.
getContact_done, so the received done signal will be ignored.

This changes the dbus macros so that we get the right signal names.

The patch above fixes this for me. But, I must be missing something because without this fix no signal with multiple words in the name works. Is this working for other people, or was this code totally untested?

Thanks for a bug report. This is quite surprising to me, because I managed couple tests at
   tests/libebook/client/...
where is also test-client-get-contact, which doesn't hang. It's a copy of the previous test for EBook, and reading it more carefully, I may admit that I expected "no bad output means test went well" which either isn't true or the issue wasn't discovered by such simple test.

Regarding your proposed change, what about not using camel-cased method names completely and use the names with underscores exclusively? I suppose it may not be a problem in GDBus, and in evolution it would clean this too, as it'll be easier to search for everything related to certain function.

Reading the code I see you've right, there are used incorrect signal names in  E_GDBUS_CONNECT_METHOD_DONE_SIGNAL_*
Let's make them with underscores, this is really confusing what to use when.

I'm wondering how this could work at all...

Thinking of it I do not expect this being caused by the signal names. Consider this all comes through glib g_signal API, thus if you would connect to a signal whose name isn't known, then you might see a runtime warning on the console. But there is no such warning, so the signal is correct.

My proof is simple, I changed the getContact to get_contact in E_DECLARE_GDBUS_ASYNC_METHOD_1_WITH_RETURN define, and then in those two below, and I get a runtime warning when I run test-client-get-contact:

> GLib-GIO-WARNING **: Trying to invoke method getContact which isn't in
> expected interface org.gnome.evolution.dataserver.AddressBook
> Failed to get contact sync: No such method `getContact'

I also added a debug print into e_gdbus_proxy_async_method_done_string(), which is responsible for the signal receiving, and it is called when I run the test program, my print is shown, 3 times, once for a sync call when checking whether the card was really added into the temporary book, once for sync test of getContact, and finally for the async call of the same.

My evolution doesn't want to freeze when I either move to address book view, or run it there. I tested with On This Computer and WebDAV addressbooks. I also can move between configured addressbooks and those expected to work (correct server address) works, contacts are fetched and shown.

Do you have any steps of how to make it hang, or any test app I would be able to use, please? (I noticed your backtrace is from evolution itself.)

Just a wild guess, are you running e-addressbook-factory yourself, on a separate console, or you let DBus run it for you? The new API expects new factories, but if, for some reason, there is still running the old factory and it "eats" the message invocation, then it will never post the done signal. Might not be some issue in this part, rather than in the eds' code itself?

You incremented the version numbers in the bus names defined in configure.ac as part of the EClient changes (AddressBook0 -> AddressBook1) so there should be no chance of D-Bus accidentally talking to the older services.  These are now completely different services as far as D-Bus knows.

I should add: as long as the client code is built against the lastest E-D-S libraries.  But I assume that's a given.

The testcase I'm running is: tests/libebook/test-self in the e-d-s tree, with a manually started (correct) e-addressbook-factory. It looks like this:

$ tests/libebook/test-self
getting the self contact
<hang>

This is what happens on the bus, via dbus-monitor (last part):

method call sender=:1.4831 -> dest=:1.4828 serial=17 path=/org/gnome/evolution/dataserver/AddressBook/13288/2; interface=org.gnome.evolution.dataserver.AddressBook; member=getContact
   string "pas-id-4DB81A1100000000"
method return sender=:1.4828 -> dest=:1.4831 reply_serial=17
   uint32 4
signal sender=:1.4828 -> dest=(null destination) serial=23 path=/org/gnome/evolution/dataserver/AddressBook/13288/2; interface=org.gnome.evolution.dataserver.AddressBook; member=get_contact_done
   uint32 4
   string ""
   string ""
   string "BEGIN:VCARD
VERSION:3.0
NICKNAME:alex
FN:Alexander Larsson
N:Larsson;Alexander;;;
X-EVOLUTION-FILE-AS:Larsson\, Alexander
UID:pas-id-4DB81A1100000000
REV:2011-04-27T13:28:49Z
END:VCARD"

I.e. we see the getContact() method call, the reply to it, and then the signal with member get_contact_done.

So, from the server side everything is correct, but we never get the result in the client. If you trace it in the client you'll see that it correctly gets the reply, but then it gets stuck in gdbus_proxy_call_sync() in the loop:

while (!e_flag_is_set (sync_data.flag)) {
	g_usleep (250000);
	g_main_context_iteration (NULL, FALSE);
}

The signal is handled in gdbus itself and ends up in gdbusproxy.c:on_signal_received() where it sees the signal name "get_contact_done" and tries to look that up with g_dbus_interface_info_lookup_signal on the proxy. However the list of signals on the proxy contains getContact_done, not get_contact_done, so we fail the lookup and silently ignore the signal.

Interesting. I see exactly the same DBus monitor lines on my machine, with respect of method names, but the test-self finishes in a second. And it doesn't matter if I run the factory from console on my own and let DBus run it for itself. I'm trying to replicate it on another machine, I'll update when I'll know the results.

It works properly on another Fedora 15 for me too (with dbus-1.4.6-4). Anyway, harmonizing the method names, like not using the camel-case, is fine for me, as I mentioned in comment #7, so I can extend your change and do that here. I'll check how it'll work for me.

How can it work for some? Maybe something changes in gdbus? I'm using git master glib.

Created attachment 188931 [details] [review]
proposed eds patch

for evolution-data-server;

I slightly extended your initial patch, to cover all camel-case GDBus identifiers and made them names with underscores. I tried to install another Fedora 15, and also Fedora 14, and I'm still unable to reproduce this issue on both of them. I'm out of idea what is the reason for the issue, or why it works for me but not for you. Could the version of glib2, DBus or gtk3 play a role here? I'm using those installed in the system.

Nonetheless, I would like to ask you for a test of this patch, whether it'll work for you. I would commit directly, but because I'm unable to reproduce this, then I'm asking you for a test. Thanks in advance.

(In reply to comment #16)
> How can it work for some? Maybe something changes in gdbus? I'm using git
> master glib.

(Oops, I overlooked your comment.) I also do not understand this, but the git master sounds like the right place to look, from my point of view, because David from bug #651099 sees similar issue as you here, and he is surely also using git master of glib and gtk.

I tried to apply the patch to my git/master eds.  It doesn't apply cleanly, and the result doesn't compile (see bug #651099).  As to Milan's guess in comment 18, I am not using git/master's for glib and gtk (versions: glib-2.29.4, gtk+-2.24.4 for gtk+2, and gtk+-3.1.4 for gtk+3.

OK, I can reproduce this with glib's git master, as of commit a588974. Some changes were necessary to have this working, due to deprecated function in it, but that's just a minor thing. I'll test the patch and commit if it'll work.

The patch works for me, thus I committed it to git master of eds. Thanks for bringing this up.

Created commit 3311169 in eds master (3.1.2+)

*** Bug 651099 has been marked as a duplicate of this bug. ***