Created attachment 336838 [details] [review]
window: Fix leak of event_creation_data struct

And also be sure to set its pointer to NULL by using g_clear_pointer()

8,830 (24 direct, 8,806 indirect) bytes in 1 blocks are
definitely lost in loss record 17,999 of 18,078
   at 0x4C2FA50: calloc (vg_replace_malloc.c:711)
   by 0x99CE981: g_malloc0 (gmem.c:124)
   by 0x99CEC64: g_malloc0_n (gmem.c:355)
   by 0x44CD43: show_new_event_widget (gcal-window.c:731)
   by 0x976FC57: ffi_call_unix64 (in /usr/lib64/libffi.so.6.0.2)
   by 0x976F6B9: ffi_call (in /usr/lib64/libffi.so.6.0.2)
   by 0x952703B: g_cclosure_marshal_generic (gclosure.c:1490)
   by 0x95258B9: g_closure_invoke (gclosure.c:804)
   by 0x9541D25: signal_emit_unlocked_R (gsignal.c:3635)
   by 0x954105C: g_signal_emit_valist (gsignal.c:3391)
   by 0x954170B: g_signal_emit_by_name (gsignal.c:3487)
   by 0x4324D8: show_popover_for_position (gcal-month-view.c:305)

Created attachment 336839 [details] [review]
e-cal-data-model: fix critical due invalid time range

libecal-CRITICAL **: e_cal_recur_generate_instances_sync: assertion
'icaltime_compare (interval_start, interval_end) < 0' failed

Attachment 336838 [details] pushed as 7dfadeb - window: Fix leak of event_creation_data struct
Attachment 336839 [details] pushed as c0ed70f - e-cal-data-model: fix critical due invalid time range

(In reply to Georges Basile Stavracas Neto from comment #3)
> Attachment 336838 [details] pushed as 7dfadeb - window: Fix leak of
> event_creation_data struct
> Attachment 336839 [details] pushed as c0ed70f - e-cal-data-model: fix
> critical due invalid time range

Did you share that last commit with eds maintainers? That code is from evolution itself.

Hi Milan,

As per comment #4, is there something to be cherry-picked here?

Hi, if you meant commit c0ed70f, then it's only a workaround. Do you know how that happened and why the start is after the end? That's obviously a bug of the caller, not of the routine where you did the change (which is common with the evolution). Note that the start time is usually inclusive, while the end time is usually exclusive, thus if you specify 2016-10-13 - 2016-10-13, then it doesn't mean whole day, the whole day is 2016-10-13 - 2016-10-14.

(In reply to Milan Crha from comment #6)
> Hi, if you meant commit c0ed70f, then it's only a workaround.

Ah, indeed.

> Do you know
> how that happened and why the start is after the end? That's obviously a bug
> of the caller, not of the routine where you did the change (which is common
> with the evolution).

Right. The whole ECalDataModel is local code. The warning was issued due both interval_start and interval_end being 0; Likely it was never set.

> Note that the start time is usually inclusive, while
> the end time is usually exclusive, thus if you specify 2016-10-13 -
> 2016-10-13, then it doesn't mean whole day, the whole day is 2016-10-13 -
> 2016-10-14.

So, even with the workaround, time (NULL) will not do what I expected it to do as it will not get the events of that day.

Thanks for clarifying, I'll re-open this bug so we can fix it properly.
It might take some time as this is not trivial to me :)

(In reply to Victor Toso from comment #7)
> The whole ECalDataModel is local code.

Just a note, that code is "shared with evolution" in a sense that it had been written for the evolution and then copied to the gnome-calendar. We try to keep them in sync, thus everyone can benefit from particular fixes.

> The warning was issued due both interval_start and interval_end being 0;
> Likely it was never set.

I see. Using <0,0> interval is forbidden since 3.20 or so, on the evolution-data-server side. It used to mean "give me everything", which is a problem for recurring events which recur forever. Once you'll find why the function is called with no time interval set (maybe it's called too early in the code), then you'll have proper fix for it. If it also means that you fire q request, which you discard shortly afterwards, then it can be a huge performance hit (consider the recurring events again). Just do not forget to revert the workaround commit, either separately, or at least when you'll have the right fix.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-calendar/issues/76.