xdsc crashes on adding a meeting

[andersk]

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.)

[geofft]

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.

[andersk]

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.

[broder]

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.

[broder]

r23643 reverts r23577, and is sitting in -proposed. Barring objections, I'll move it to production late Tuesday afternoon.

Someone confirming that it's no more broken than what's currently in production would be nice.

[broder]

I moved the reverted package to production, so I'm stripping the proposed tag.

[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.

[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).

[jdreed]

 https://github.com/mit-athena/xdsc/pull/1

[jdreed]

Note that this requires the latest changes to python-discuss.

[jdreed]

OK, added Debian packaging for this, and merged master into debian on my checkout.

 https://github.com/mit-athena/xdsc/pull/2