Closed
Bug 305857
Opened 19 years ago
Closed 19 years ago
icalComponent-related crashes
Categories
(Calendar :: General, defect)
Calendar
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: gekacheka, Assigned: dmosedale)
References
Details
Attachments
(3 files, 1 obsolete file)
(deleted),
text/calendar
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
mvl
:
first-review+
|
Details | Diff | Splinter Review |
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?)
Comment 2•19 years ago
|
||
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
Assignee | ||
Comment 4•19 years ago
|
||
Seems like a plausible explanation to me. Wanna try putting together a patch
for the calICSService and see if that fixes it?
Comment 5•19 years ago
|
||
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.)
Comment 7•19 years ago
|
||
(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.
Assignee | ||
Comment 8•19 years ago
|
||
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
Assignee | ||
Comment 9•19 years ago
|
||
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.
Assignee | ||
Comment 10•19 years ago
|
||
(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?
Assignee | ||
Comment 11•19 years ago
|
||
*** Bug 303922 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 12•19 years ago
|
||
*** Bug 311904 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 13•19 years ago
|
||
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
Assignee | ||
Comment 14•19 years ago
|
||
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.
Assignee | ||
Comment 15•19 years ago
|
||
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.
Assignee | ||
Comment 16•19 years ago
|
||
Attachment #201189 -
Attachment is obsolete: true
Attachment #201278 -
Flags: first-review?(mvl)
Comment 17•19 years ago
|
||
Comment on attachment 201278 [details] [diff] [review]
stop manually adding VTIMEZONEs
r=mvl
Attachment #201278 -
Flags: first-review?(mvl) → first-review+
Assignee | ||
Comment 18•19 years ago
|
||
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.
Description
•