Re: [libvirt] PATCH] Stop double free

Mark Hamzy wrote: > The stack trace is as follows: > > Program received signal SIGABRT, Aborted. > 0x00000035ad830265 in raise () from /lib64/libc.so.6 > (gdb) bt > #0 0x00000035ad830265 in raise () from /lib64/libc.so.6 > #1 0x00000035ad831d10 in abort () from /lib64/libc.so.6 > #2 0x00000035ad86a84b in __libc_message () from /lib64/libc.so.6 > #3 0x00000035ad8722ef in _int_free () from /lib64/libc.so.6 > #4 0x00000035ad87273b in free () from /lib64/libc.so.6 > #5 0x0000000000406771 in vshDeinit (ctl=0x7fffd35d35e0) at virsh.c:8244 > #6 0x00000000004069a5 in vshError (ctl=0x7fffd35d35e0, doexit=<value > optimized out>, format=0x414f66 "%s") at virsh.c:7861 > #7 0x00000000004067c4 in vshDeinit (ctl=0x7fffd35d35e0) at virsh.c:8248 > #8 0x000000000041335e in main (argc=3, argv=0x7fffd35d3748) at virsh.c:8493 > > I am trying to run libvirt-0.7.1-0.1.git3ef2e05.fc12.src.rpm on RHEL5.4. > > vshDeinit gets called twice, so ctl->name is freed twice. > > How about this patch then? > Ah, I see now. Your patch is a workaround. The real problem is that vshDeinit is re-entering itself through: vshDeinit()->vshError()->vshDeinit() While your patch would fix the problem, I'm not sure it's a good long-term solution. Other differences might come up in the future, and trying to worry about vshDeinit being re-entrant is probably not worth the effort. (Indeed, it looks like there were earlier attempts to avoid this, but things have changed since then, breaking the workaround). I think we should make it so that vshDeinit() does not try to re-enter itself. At the moment I don't have a patch, but I would look at either splitting vshError() into vshPrintError() and vshError(), or just doing a couple of fprintf()'s directly in vshDeinit() and not calling vshError() at all (with a comment explaining why).