Closed Bug 305857 Opened 19 years ago Closed 19 years ago

icalComponent-related crashes

Categories

(Calendar :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gekacheka, Assigned: dmosedale)

References

Details

Attachments

(3 files, 1 obsolete file)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20050824 Mozilla Sunbird/0.2+ Sunbird crashes deleting item from file. Most apparent difference is that this file has timezone and dates that refer to it. Reproducible: Always Steps to Reproduce: 1. Save iCalendar text to local file c:/temp/ICSTest.ics (with crlf newlines) 2. File | new calendar | remote | webdav, file:///c:/temp/ICSTest.ics, name ICSTest 3. event "event with timezone" should appear on 20050825 4. select and delete it. Actual Results: Sunbird crashed. The instruction at "0x00829843" referenced memory at "0x00000005". The memory could not be "read". Expected Results: No crash, and the event deleted. (Need bugzilla component for ics provider?)
I have seen sunbird crash, when doing operations related to timezones. But i couldn't find steps to reproduce. Your testcase aslo worksforme. I can delete the item just fine. no crash. (on linux)
Crash deleting event with timezone seems to be occurring during call to calICSService.serializeToICS() from calICSCalendar.writeICS() at http://lxr.mozilla.org/mozilla/source/calendar/providers/ics/calICSCalendar.js#231 At this call, the calICSCalendar has a VTIMEZONE (added from this.unmappedComponents), and no other subcomponents. (Seems correct to keep the timezone in unmappedComponents in case it is referenced in other unmapped components or properties.) (Speculating via lxr: calIcalComponent::SerializeToICS appears to implement the call. http://lxr.mozilla.org/mozilla/source/calendar/base/src/calICSService.cpp#781 The first thing serializeToICS does is "add timezone bits", calling AddTimezoneComponentToIcal. http://lxr.mozilla.org/mozilla/source/calendar/base/src/calICSService.cpp#781 It seems to call icalcomponent_merge_component with (vcalendar, VTIMEZONE). http://lxr.mozilla.org/mozilla/source/calendar/base/src/calICSService.cpp#769 But icalendarcomponent_merge_component takes (vcalendar, VCALENDAR) http://lxr.mozilla.org/mozilla/source/calendar/libical/src/libical/icalcomponent.c#2099 So maybe assert fails? Would that appear as crash?)
Version: unspecified → Trunk
Seems like a plausible explanation to me. Wanna try putting together a patch for the calICSService and see if that fixes it?
IF that would indeed be the cause of the crash, why does serializing usually work just fine? Only in very spacial cases, it dies.
(In reply to comment #5) > IF that would indeed be the cause of the crash, why does serializing usually > work just fine? Only in very spacial cases, it dies. Are there VTIMEZONE elements? In my examinations of the ics file, for events created with the current eventDialog, there hasn't been a VTIMEZONE element. Instead all the dates were zulu. (I have the "store all dates in universal time" option turned off, so I'm not sure why they are stored in zulu.) (If you want to test with more than the above file, the item dialog extension in bug 298102 gets each date in the preferred timezone. After creating an event with this item dialog, each of its dates appears with a timezone id in the .ics file, and a VTIMEZONE element also appears for the timezone.)
(In reply to comment #6) > Are there VTIMEZONE elements? I tried it with the testcase attached to this bug. No assertion and no crash. The event is deleted properly.
For some time, I've been seeing lots of crashes when dealing with icalComponents. Often, these happen when adding a subcomponent. Valgrind info and stack forthcoming.
OS: Windows 2000 → All
Hardware: PC → All
Summary: crashes deleting item from ics file with timezone → icalComponent-related crashes
Attached file valgrind output for one crash (deleted) —
Once, I was able to catch one of these crashes using valgrind --tool=addrcheck (I haven't yet been able to reproduce one while running with --tool=memcheck). Attaching the relevant output up to the point of the crash.
(In reply to comment #9) > Created an attachment (id=197701) [edit] > valgrind output for one crash > > Once, I was able to catch one of these crashes using valgrind --tool=addrcheck > (I haven't yet been able to reproduce one while running with --tool=memcheck). > Attaching the relevant output up to the point of the crash. Eyeballing that log, it appears that things start going bad somewhat before we get to the point of actually crashing. My interpretation of the first valgrind warning is that somebody must have passed in a corrupt child component to calIcalComponent::AddSubcomponent. So the interesting question becomes: where did this corrupt component originate? Unless we're seeing generalized heap-stomping, presumably the component was generated earlier on in calICSService.cpp. Shaver, do you have any ideas or speculation on how this might have happened?
*** Bug 303922 has been marked as a duplicate of this bug. ***
*** Bug 311904 has been marked as a duplicate of this bug. ***
There are a couple of problems here. One has to do with a bug in calICSService::AddSubcomponent, and the other is apparently a bug in libical having to do with merging in timezone components.
Assignee: mostafah → dmose
Attached patch work-in-progress, v1 (obsolete) (deleted) — Splinter Review
In the current code, AddSubcomponent attempts to reparent the passed in sub-component. This has the problem that the calendar to which it originally belonged still thinks that it (also) has ownership. This patch fixes that and adds some debugging and logging code. Interesting, with this patch applied, the second problem crops up on essentially every calendar modification rather than just occasionally. I'm on the trail of the second bug.
I've now spun off bug 314334, bug 314337, and bug 314339 about issues relevant to the crashes going on here. I believe that we can fix the most basic and common cause of this bug by just having the calICSCalendar provider not manually add VTIMEZONE components, at which point we can close this bug and worry about the other issues separately.
Attached patch stop manually adding VTIMEZONEs (deleted) — Splinter Review
Attachment #201189 - Attachment is obsolete: true
Attachment #201278 - Flags: first-review?(mvl)
Comment on attachment 201278 [details] [diff] [review] stop manually adding VTIMEZONEs r=mvl
Attachment #201278 - Flags: first-review?(mvl) → first-review+
Fix checked in.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: