[ctwm] repository has a double-free() bug
I'm working on getting changes to the virtual workspaces pushed up and
I found a bug in the recent memory management cleanups where memory
is being freed twice. The attached patch against cvs should comment
out the code causing the issue, but I have yet to track down what the
right answer is to free memory when it should be freed, but not when it
shouldn't.
The double free was actually causing a seg fault that appeared to be
stomping on memory such that I was getting a non-sensical crash dump in
gdb. (This is under NetBSD/i386, if it matters; it also keeps track and
notifies of double-memory frees).
I was able to easily trigger this after opening and closing a few xterms
and quickly shifting focus into it. I could quite figure out the exact
things that were triggering it beyond open/close/dealing with focus, but
it seemed like if I opened a window and let it sit there for a while (20
secs?), then I wouldn't crash it by shifting focus around.
I'm not going to commit this patch since it probably reintroduces a
memory leak, but figured I'd share what I found. For now, the ctwm that
I'm running is based on the HEAD of the tree but with this patch (plus
changes I'm testing before commit).
note that this is against the cvs repository, and 3.7 does not suffer
from this.
-Todd
--- add_window.cefc8288ef604525373100ce0eb1e04654bc7e6d4
+++ add_window.c3f9659833e74eda8aebd20120b2defc3649af8d9
@@ -1764,7 +1766,15 @@
} else {
XFreePixmap (dpy, tmp_win-HiliteImage-pixmap);
}
+#if 0
+ /*
+* XXX - this was being free'd twice for some reason. Commenting
+* this out almost certainly introduces a memory leak, but they're
+* better than core dumps. :)
+*/
free(tmp_win-HiliteImage);
+ tmp_win-HiliteImage = NULL;
+#endif
}
}
[ctwm] Re: repository has a double-free() bug
OK, I've a question: why do you remove the code entirely? presumably this is supposed to get freed at some point, just not twice, so I left it in there for someone to try to dig into it (presumably the original commiter/patch provider). Near as I can tell, this code is the only code that actually frees that value, but it wasn't clear from looking at it if I could/should just do away with it or if the free should only be called under certain circumstances. Didn't it help enough to make tmp_win-HiliteImage NULL? no. It still ended up freeing memory that was already freed. I'm not quite clear on where it got the value from to pass in after setting it to NULL. I'm deducing this from looking at the core dump of running (under bash on NetBSD): MALLOC_OPTIONS='A' ctwm which forces a core dump when the double free is attempted. gdb of the core dump indicated the double-free was at the free(tmp_win-HiliteImage); This definately perplexed and continues to perplex me, though I didn't dig into where HiliteImage is allocated too deeply. -Todd
[ctwm] Re: repository has a double-free() bug
In message [EMAIL PROTECTED] on Sun, 21 May 2006 14:42:00 -0400, Todd Kover [EMAIL PROTECTED] said: kovert Didn't it help enough to make tmp_win-HiliteImage NULL? kovert kovert no. It still ended up freeing memory that was already freed. kovert I'm not quite clear on where it got the value from to pass in kovert after setting it to NULL. Wait, so you're saying that after you set it to NULL, some other value is placed there? Aha, I think I know why. In events.c, HandleDestroyNotify doesn't set Tmp_win to NULL, which means that the next call to it will try to delete whatever it points at. In the mean time, that are of memory has most likely been reallocated, making Tmp_win point at apparently bogus data, most probably leading to a crash! So it's quite likely that the following patch fixes this problem. Can you verify that (remove your #id 0 and #endif)? --- events.cbf62739b47179668fa7b5aa47c35af1e4be19c64 +++ events.c69f6f0e64e740e88fd1ec2899c7c158c27a9289a @@ -2335,6 +2335,7 @@ DeleteIconsList (Tmp_win); /* 14 */ free((char *)Tmp_win); +Tmp_Win = NULL; if (Scr-ClickToFocus || Scr-SloppyFocus) set_last_window (Scr-currentvs-wsw-currentwspc); I think we need to make an effort to check that we reset free'd things with a NULL everywhere that happens. Cheers, Richard - Please consider sponsoring my work on free software. See http://www.free.lp.se/sponsoring.html for details. -- Richard Levitte [EMAIL PROTECTED] http://richard.levitte.org/ When I became a man I put away childish things, including the fear of childishness and the desire to be very grown up. -- C.S. Lewis
[ctwm] Re: Will upgrade monotone some time next week
On Sat, May 20, 2006 at 12:48:45PM +0200 I heard the voice of Richard Levitte - VMS Whacker, and lo! it spake thus: If you have any question, please ask and I'll help you as much as I can. I took this as an opportunity to try this again; I've had horrible luck getting monotone to work at all, finally tracked down to b0rkedness in the boost libraries. The problem now, I find, is that the database somehow seems to still want to use repository.lp.se as the server to connect to when I do a `mtn pull` from the working dir. Since I know I didn't do the initial pull from there, I presume it's inheriting that info somehow, but I'm not sure how to change it. The online help and manpage don't seem to give me any hints... -- Matthew Fuller (MF4839) | [EMAIL PROTECTED] Systems/Network Administrator | http://www.over-yonder.net/~fullermd/ On the Internet, nobody can hear you scream.
[ctwm] Re: Will upgrade monotone some time next week
In message [EMAIL PROTECTED] on Mon, 22 May 2006 02:51:07 +0200 (CEST), Richard Levitte - VMS Whacker [EMAIL PROTECTED] said: richard I'm asking you to put it aside and start with a completely richard new a fresh database. It dawned on me that I haven't explained why. The following section in the manual should explain it well enough (if you consider that db rosterify is like another db rebuild): http://www.venge.net/monotone/docs/Rebuilding-ancestry.html#Rebuilding-ancestry Cheers, Richard - Please consider sponsoring my work on free software. See http://www.free.lp.se/sponsoring.html for details. -- Richard Levitte [EMAIL PROTECTED] http://richard.levitte.org/ When I became a man I put away childish things, including the fear of childishness and the desire to be very grown up. -- C.S. Lewis
