Ticket #84 (closed defect: fixed)
xdsc crashes on adding a meeting
Reported by: | andersk | Owned by: | |
---|---|---|---|
Priority: | trivial | Milestone: | The Distant Future |
Component: | -- | Keywords: | |
Cc: | Fixed in version: | ||
Upstream bug: |
Description
After adding the sipb-ec meeting with xdsc on linerva, it crashed with
*** glibc detected *** corrupted double-linked list: 0xf7de74f8 ***
(Fortunately, the meeting was successfully added.)
Change History
comment:2 Changed 16 years ago by andersk
- Status changed from closed to reopened
- Resolution fixed deleted
debathena / r23577 / andersk 14:14 (Anders Kaseorg)
It seems pretty clear to me that there are no pointers to those
tempstrings lying around after the free()s you commented. I’m
guessing that something else thinks it’s using the same space. Did
you try valgrind?
debathena / r23577 / andersk 14:15 (Anders Kaseorg)
Replacing an easy-to-reproduce crash with a hard-to-reproduce subtle
memory corruption bug is not an improvement.
comment:3 Changed 16 years ago by broder
- Keywords proposed added
Tagging as "proposed" because there currently is a package in -proposed.
However, I'm convinced by what Anders says that this patch doesn't understand and fully fix the issue, so unless someone is planning to fix this in the near future, I think we should revert r23577 and upload another release that we can move into production.
comment:5 Changed 16 years ago by broder
- Keywords proposed removed
- Component set to default
I moved the reverted package to production, so I'm stripping the proposed tag.
comment:6 Changed 16 years ago by jdreed
- Priority changed from minor to low
- Component set to --
- Milestone set to The Distant Future
comment:7 Changed 13 years ago by jdreed
So, I went and ran valgrind on xdsc, and based on my (possibly naive) interpretation of the output, it looks like Geoff's patch was right?
==21275== Invalid free() / delete / delete[] ==21275== at 0x4C282ED: free (vg_replace_malloc.c:366) [snip] ==21275== Address 0x701e750 is 0 bytes inside a block of size 17 free'd ==21275== at 0x4C282ED: free (vg_replace_malloc.c:366) ==21275== by 0x4043E4: myfree (xdsc.c:1225) ==21275== by 0x4071AD: AddCB (reply.c:715)
Memory management in C is definitely not my thing, though. Unless there's any real interest in fixing this, I'm just going to go write a pygtk version.
comment:8 Changed 12 years ago by jdreed
Out of curiousity, I poked at this some more. I am not convinced geofft's patch was wrong. Not only does valgrind back me up, but buried deep within the Motif programming manual is a note that "An application should free compound strings retrieved by XtGetValues?, but in general it should not free values of other types unless the documentation for the particular resource [...] says the application must free that value." And in fact the documentation does not say the address passed to XtSetArg? must be free'd. (We're not using Motif compound strings here).
comment:10 Changed 11 years ago by jdreed
- Status changed from reopened to review
Note that this requires the latest changes to python-discuss.
comment:11 Changed 11 years ago by jdreed
OK, added Debian packaging for this, and merged master into debian on my checkout.
comment:12 Changed 10 years ago by jdreed
- Status changed from review to closed
- Resolution set to fixed
The crash also happens when removing a meeting.
I've avoided the crash by commenting out three calls to free in reply.c (see r23577). I'm not sure if I'm actually avoiding a double-free, or just avoiding some stupidity and instead leaking memory, but a potential memory leak of a short string every time you add or remove a meeting is very hard for me to care about.
Someone should rewrite the whole thing in Gtk+. Or, you know, write ds2imap or ds2nntp so I can just use my favorite mail/news reader.