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 655986 - file backend: e_cal_create/remove_object problems
file backend: e_cal_create/remove_object problems
Status: RESOLVED FIXED
Product: evolution-data-server
Classification: Platform
Component: Calendar
2.32.x (obsolete)
Other Windows
: Normal normal
: ---
Assigned To: evolution-calendar-maintainers
Evolution QA team
Depends on:
Blocks:
 
 
Reported: 2011-08-04 15:52 UTC by Patrick Ohly
Modified: 2011-08-10 16:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ecal file backend: avoid manipulating the UID inside component_add() (5.31 KB, patch)
2011-08-04 19:37 UTC, Patrick Ohly
none Details | Review
calendar file backend: fixed incomplete sanity check in e_cal_create_object() (2.60 KB, patch)
2011-08-04 19:38 UTC, Patrick Ohly
none Details | Review
libecal: e_cal_remove_object() must remove *all* recurrences (2.18 KB, patch)
2011-08-04 19:38 UTC, Patrick Ohly
none Details | Review
[PATCH 1/3] ecal file backend: avoid manipulating the UID inside component_add() [MASTER] (5.31 KB, patch)
2011-08-09 08:50 UTC, Christophe Dumez
committed Details | Review
[PATCH 2/3] calendar file backend: fixed incomplete sanity check in e_cal_create_object() [MASTER] (2.28 KB, patch)
2011-08-09 08:51 UTC, Christophe Dumez
committed Details | Review
[PATCH 3/3] libecal: e_cal_remove_object() must remove *all* recurrences [MASTER] (3.24 KB, patch)
2011-08-09 08:53 UTC, Christophe Dumez
committed Details | Review

Description Patrick Ohly 2011-08-04 15:52:06 UTC
SyncEvolution 1.1.99.5 + Evolution Data Server from gnome_2_32 branch (8127525268c617367b8a443c203f468280399cd2).

Run Client::Source::eds_event::testLinkedItemsChildParent. e_cal_get_object() and thus the test fail with "Object not found" although the UID is one reported by e_cal_get_object_list() earlier.

This occurs because the internal hash contains an entry where the key doesn't match the UID of the component contained in it: e_cal_get_object_list() reports the UID, which then isn't found in the hash.

valgrind on e-calendar-factory:

==10069== Invalid read of size 1
==10069==    at 0x4C25812: __GI_strlen (mc_replace_strmem.c:284)
==10069==    by 0x8EF011E: g_strdup (gstrfuncs.c:99)
==10069==    by 0xF4E08B6: e_cal_backend_file_create_object (e-cal-backend-file.c:2363)
==10069==    by 0x93E6061: e_cal_backend_sync_create_object (e-cal-backend-sync.c:214)
==10069==    by 0x93E86D3: _e_cal_backend_create_object (e-cal-backend-sync.c:630)
==10069==    by 0x93DD40B: e_cal_backend_create_object (e-cal-backend.c:1017)
==10069==    by 0x93F0C34: impl_Cal_createObject (e-data-cal.c:401)
==10069==    by 0x4E75383: _e_gdbus_gdbus_cclosure_marshaller_BOOLEAN__OBJECT_STRING (e-gdbus-marshallers.c:377)
==10069==    by 0x820999E: g_closure_invoke (gclosure.c:773)
==10069==    by 0x8225972: signal_emit_unlocked_R (gsignal.c:3256)
==10069==    by 0x82248D0: g_signal_emit_valist (gsignal.c:2997)
==10069==    by 0x8224DBC: g_signal_emit (gsignal.c:3044)
==10069==  Address 0x1499c7b0 is 0 bytes inside a block of size 39 free'd
==10069==    at 0x4C240FD: free (vg_replace_malloc.c:366)
==10069==    by 0x9DE952C: icalvalue_free (in /usr/lib/libical.so.0.44.0)
==10069==    by 0x9DDB796: icalproperty_set_value (in /usr/lib/libical.so.0.44.0)
==10069==    by 0x4E4FFA2: e_cal_component_set_uid (e-cal-component.c:1479)
==10069==    by 0xF4DB8F3: check_dup_uid (e-cal-backend-file.c:498)
==10069==    by 0xF4DBD9B: add_component (e-cal-backend-file.c:614)
==10069==    by 0xF4E0894: e_cal_backend_file_create_object (e-cal-backend-file.c:2356)
==10069==    by 0x93E6061: e_cal_backend_sync_create_object (e-cal-backend-sync.c:214)
==10069==    by 0x93E86D3: _e_cal_backend_create_object (e-cal-backend-sync.c:630)
==10069==    by 0x93DD40B: e_cal_backend_create_object (e-cal-backend.c:1017)
==10069==    by 0x93F0C34: impl_Cal_createObject (e-data-cal.c:401)
==10069==    by 0x4E75383: _e_gdbus_gdbus_cclosure_marshaller_BOOLEAN__OBJECT_STRING (e-gdbus-marshallers.c:377)

An explanation of how this happens will follow in the commit messages for the patches fixing the issue.
Comment 1 Patrick Ohly 2011-08-04 19:27:48 UTC
I have prepared patches against the gnome_2_32 branch, which is what I can test with at the moment. The same problems exist in the master and gnome_3_0 branches. I have not tested with a non-file backend.

It would be good if someone with access to different backends could verify that e_cal_remove_object() with MOD_ALL works as intended.
Comment 2 Patrick Ohly 2011-08-04 19:37:36 UTC
Created attachment 193274 [details] [review]
ecal file backend: avoid manipulating the UID inside component_add()
Comment 3 Patrick Ohly 2011-08-04 19:38:03 UTC
Created attachment 193275 [details] [review]
calendar file backend: fixed incomplete sanity check in e_cal_create_object()
Comment 4 Patrick Ohly 2011-08-04 19:38:27 UTC
Created attachment 193276 [details] [review]
libecal: e_cal_remove_object() must remove *all* recurrences
Comment 5 Christophe Dumez 2011-08-09 08:50:39 UTC
Created attachment 193467 [details] [review]
[PATCH 1/3] ecal file backend: avoid manipulating the UID inside  component_add() [MASTER]
Comment 6 Christophe Dumez 2011-08-09 08:51:33 UTC
Created attachment 193468 [details] [review]
[PATCH 2/3] calendar file backend: fixed incomplete sanity check in  e_cal_create_object() [MASTER]
Comment 7 Christophe Dumez 2011-08-09 08:53:06 UTC
Created attachment 193469 [details] [review]
[PATCH 3/3] libecal: e_cal_remove_object() must remove *all*  recurrences [MASTER]

Ported all the patches to the master branch and updated the documentation accordingly (the new ECalClient documentation was conflicting with the change).
Comment 8 Christophe Dumez 2011-08-09 13:22:44 UTC
I can confirm that EDS master is also affected by the issue and that the attached patches make the Client::Source::eds_event tests pass.
Comment 9 Milan Crha 2011-08-10 13:44:52 UTC
I do not see any change while trying a test from evolution's UI, it seems to do the same with or without the patch, thus yes, please commit to master. (I tested master versions only, though feel free to commit to the 2.32 branch too.) I suppose it's because evolution is using views,which don't suffer of this inconsistency.
Comment 10 Milan Crha 2011-08-10 13:47:59 UTC
One thing I was afraid of was that the add_component is called from multiple places, but only on one you added the new UID generation. That's kinda strange and can, probably, lead to other issues, but as I wasn't able to reproduce it under calendar evolution UI, then I'm most likely wrong.
Comment 11 Patrick Ohly 2011-08-10 16:15:19 UTC
Applied the patches to master and cherry-picked into gnome-3-0 and gnome-2-32.