After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 693464 - backend-property-changed signal never received
backend-property-changed signal never received
Status: RESOLVED FIXED
Product: evolution-data-server
Classification: Platform
Component: general
3.8.x (obsolete)
Other Linux
: Normal critical
: ---
Assigned To: Evolution Shell Maintainers Team
Evolution QA team
Depends on:
Blocks:
 
 
Reported: 2013-02-09 06:40 UTC by Milan Crha
Modified: 2015-02-18 15:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
test.c (2.79 KB, text/x-chdr)
2013-02-09 06:40 UTC, Milan Crha
Details
eds test patch (949 bytes, text/plain)
2013-02-11 10:02 UTC, Milan Crha
Details
updated test.c (3.07 KB, text/x-chdr)
2013-02-11 10:04 UTC, Milan Crha
Details

Description Milan Crha 2013-02-09 06:40:56 UTC
Created attachment 235579 [details]
test.c

Checkout the test.c from the attachment and compile it. The both books should receive "backend-property-changed" signal, but it is received in one only.
The order doesn't matter.

I do not think I do anything insane in the test, neither unusual, hence the blocker status due to wrong design.
Comment 1 Matthew Barnes 2013-02-09 13:15:21 UTC
This is because "backend-property-changed" signals are received over D-Bus first and scheduled to be emitted from idle callbacks, followed by the asynchronous open() being scheduled to complete from an idle callback.  So by the time you call e_book_client_connect_finish(), you've already missed the initial round of "backend-property-changed" signals.

Whereas with the synchronous open(), the idle callbacks for the signal emissions have not yet run since we're still in the same main loop iteration.

Subsequent property changes *will* trigger a signal emission, which is all that really matters.  Your test shows the initial property states after either open() method are correct, so I don't see what the bug is.
Comment 2 Milan Crha 2013-02-11 10:02:25 UTC
(In reply to comment #1)
> This is because "backend-property-changed" signals are received over D-Bus
> first and scheduled to be emitted from idle callbacks, followed by the
> asynchronous open() being scheduled to complete from an idle callback.  So by
> the time you call e_book_client_connect_finish(), you've already missed the
> initial round of "backend-property-changed" signals.

The description only proves me that the code is over complicated.

> Subsequent property changes *will* trigger a signal emission, which is all that
> really matters.

Do you think that, or you know that? These are two different things.
I do not have any simple reproducer, thus only when I'll pretend a read-only property change on the local book with the below patch then I get an answer for my question.
Comment 3 Milan Crha 2013-02-11 10:02:54 UTC
Created attachment 235689 [details]
eds test patch

eds test patch, apply and rerun the book factory
Comment 4 Milan Crha 2013-02-11 10:04:55 UTC
Created attachment 235690 [details]
updated test.c

This should print
> book_backend_property_changed_cb: readonly
4 times, twice for each book, but it prints it only twice after the 'revision' property change.
Comment 5 Tristan Van Berkom 2013-03-16 07:39:32 UTC
Note, this causes libecal test-client-get-revision to break.

The test adds components in a loop, each iteration expects a new
revision to be created in response to the added component, but it
quickly breaks.

Either the client never receives the notification, or there is a bad
race condition (somehow this does not happen for the equivalent libebook
test, which basically does exactly the same thing, so that's why i wonder
if it's actually a race condition, perhaps the calendar backend reacts
slightly faster).
Comment 6 Tristan Van Berkom 2013-04-24 08:55:20 UTC
Note: test-client-get-revision for calendars is not broken anymore, so I just
re-enabled it in the suite (had it commented out for this).

Not sure exactly how it was fixed but I think it has to do with Matthew's
recent fix related to the calendar leaking when setting the revision.
Comment 7 Milan Crha 2015-02-18 15:30:11 UTC
I figured out what the problem is. It's the EAsyncClosure and the fact the an EClient references a thread-default context when it is created, which is the EAsyncClosure's main context, but the EAsyncClose is supposed to die shortly after the book is opened. The code was quite similar to the used in e_client_cache_get_client_sync().

I made changes for 3.13.90 to address this, which also avoids memory leaks, in commit_1ad40c64a74ee in evolution [1], thus I'm closing this.

[1] https://git.gnome.org/browse/evolution/commit/?id=1ad40c64a74ee