Bug#429245: libguichan0: Segfaults when destroying dropdown widgets

[Patrick Matthäi]

Thank you very much, I included the patch and the update will come soon.

I posted it to the guichan forum, too:

http://guichan.sourceforge.net/forum/read.php?2,1805

It will be included with a little change in the next upstream release.

Regards,
   Patrick Matthäi

Bug#429245: libguichan0: Segfaults when destroying dropdown widgets

[Patrick Matthäi]

Guillaume Melquiond schrieb:

I have hit another segfault while destroying dropdown widgets. This
time, it is due to the internal focus handler being automatically
destroyed by DropDown::~DropDrow yet still used later by the inherited
destructors. So I attach a new patch. In addition to the bugfix
contained in the previous patch, it removes the internal focus handler
while it is still alive.
  

Hello Melquiond,

thanks for your patch! I will test it around these days. Do you have 
maybe another system with 32bit and can try to reproduce this crash with 
it? Because on my 32bit architecture it's running well.


And please add a gdb tracelog :)

Regards,
   Patrick Matthäi

Bug#429245: libguichan0: Segfaults when destroying dropdown widgets

[Guillaume Melquiond]

Le dimanche 17 juin 2007 à 12:08 +0200, Patrick Matthäi a écrit :

 thanks for your patch! I will test it around these days. Do you have 
 maybe another system with 32bit and can try to reproduce this crash with 
 it? Because on my 32bit architecture it's running well.

No, sorry.

But this doesn't surprise me much you don't encounter a segfault:
Everything happens during the destruction of a dropdown widget, so the
system is in a sane state before the destruction starts, and it may
still be after the destruction ends. So you have to be quite unlucky to
get corrupted memory during this short lapse of time.

 And please add a gdb tracelog :)

Better than a gdb trace, here comes a valgrind trace.

Invalid read of size 8
   at 0x61FC5CE: gcn::FocusHandler::remove(gcn::Widget*) (in 
/usr/lib/libguichan.so.0.0.0)
   by 0x6204CE0: gcn::Widget::_setFocusHandler(gcn::FocusHandler*) (in 
/usr/lib/libguichan.so.0.0.0)
   by 0x61FA46D: gcn::BasicContainer::_setFocusHandler(gcn::FocusHandler*) (in 
/usr/lib/libguichan.so.0.0.0)
   by 0x61FA6CE: gcn::BasicContainer::clear() (in /usr/lib/libguichan.so.0.0.0)
   by 0x61FA950: gcn::BasicContainer::~BasicContainer() (in 
/usr/lib/libguichan.so.0.0.0)
   by 0x620A560: gcn::DropDown::~DropDown() (in /usr/lib/libguichan.so.0.0.0)
   by 0x41B3E0: DropDown::~DropDown() (dropdown.cpp:96)
   by 0x442F91: GCContainer::~GCContainer() (gccontainer.cpp:34)
   by 0x4995E9: Window::~Window() (window.cpp:157)
   by 0x469E20: ServerDialog::~ServerDialog() (serverdialog.cpp:194)
   by 0x4DB560: main (main.cpp:820)
 Address 0xBD6F208 is 0 bytes inside a block of size 16 free'd
   at 0x4A1F37C: operator delete(void*) (vg_replace_malloc.c:244)
   by 0x620A533: gcn::DropDown::~DropDown() (in /usr/lib/libguichan.so.0.0.0)
   by 0x41B3E0: DropDown::~DropDown() (dropdown.cpp:96)
   by 0x442F91: GCContainer::~GCContainer() (gccontainer.cpp:34)
   by 0x4995E9: Window::~Window() (window.cpp:157)
   by 0x469E20: ServerDialog::~ServerDialog() (serverdialog.cpp:194)
   by 0x4DB560: main (main.cpp:820)

Invalid read of size 1
   at 0x4A20B10: memmove (mc_replace_strmem.c:514)
   by 0x61FD49B: std::vectorgcn::Widget*, std::allocatorgcn::Widget* 
::erase(__gnu_cxx::__normal_iteratorgcn::Widget**, std::vectorgcn::Widget*, 
std::allocatorgcn::Widget*  ) (in /usr/lib/libguichan.so.0.0.0)
   by 0x6204CE0: gcn::Widget::_setFocusHandler(gcn::FocusHandler*) (in 
/usr/lib/libguichan.so.0.0.0)
   by 0x61FA46D: gcn::BasicContainer::_setFocusHandler(gcn::FocusHandler*) (in 
/usr/lib/libguichan.so.0.0.0)
   by 0x61FA6CE: gcn::BasicContainer::clear() (in /usr/lib/libguichan.so.0.0.0)
   by 0x61FA950: gcn::BasicContainer::~BasicContainer() (in 
/usr/lib/libguichan.so.0.0.0)
   by 0x620A560: gcn::DropDown::~DropDown() (in /usr/lib/libguichan.so.0.0.0)
   by 0x41B3E0: DropDown::~DropDown() (dropdown.cpp:96)
   by 0x442F91: GCContainer::~GCContainer() (gccontainer.cpp:34)
   by 0x4995E9: Window::~Window() (window.cpp:157)
   by 0x469E20: ServerDialog::~ServerDialog() (serverdialog.cpp:194)
   by 0x4DB560: main (main.cpp:820)
 Address 0xBD6F210 is 8 bytes inside a block of size 16 free'd
   at 0x4A1F37C: operator delete(void*) (vg_replace_malloc.c:244)
   by 0x620A533: gcn::DropDown::~DropDown() (in /usr/lib/libguichan.so.0.0.0)
   by 0x41B3E0: DropDown::~DropDown() (dropdown.cpp:96)
   by 0x442F91: GCContainer::~GCContainer() (gccontainer.cpp:34)
   by 0x4995E9: Window::~Window() (window.cpp:157)
   by 0x469E20: ServerDialog::~ServerDialog() (serverdialog.cpp:194)
   by 0x4DB560: main (main.cpp:820)

Invalid write of size 1
   at 0x4A20B14: memmove (mc_replace_strmem.c:514)
   by 0x61FD49B: std::vectorgcn::Widget*, std::allocatorgcn::Widget* 
::erase(__gnu_cxx::__normal_iteratorgcn::Widget**, std::vectorgcn::Widget*, 
std::allocatorgcn::Widget*  ) (in /usr/lib/libguichan.so.0.0.0)
   by 0x6204CE0: gcn::Widget::_setFocusHandler(gcn::FocusHandler*) (in 
/usr/lib/libguichan.so.0.0.0)
   by 0x61FA46D: gcn::BasicContainer::_setFocusHandler(gcn::FocusHandler*) (in 
/usr/lib/libguichan.so.0.0.0)
   by 0x61FA6CE: gcn::BasicContainer::clear() (in /usr/lib/libguichan.so.0.0.0)
   by 0x61FA950: gcn::BasicContainer::~BasicContainer() (in 
/usr/lib/libguichan.so.0.0.0)
   by 0x620A560: gcn::DropDown::~DropDown() (in /usr/lib/libguichan.so.0.0.0)
   by 0x41B3E0: DropDown::~DropDown() (dropdown.cpp:96)
   by 0x442F91: GCContainer::~GCContainer() (gccontainer.cpp:34)
   by 0x4995E9: Window::~Window() (window.cpp:157)
   by 0x469E20: ServerDialog::~ServerDialog() (serverdialog.cpp:194)
   by 0x4DB560: main (main.cpp:820)
 Address 0xBD6F208 is 0 bytes inside a block of size 16 free'd
   at 0x4A1F37C: operator delete(void*) (vg_replace_malloc.c:244)
   by 0x620A533: gcn::DropDown::~DropDown() (in /usr/lib/libguichan.so.0.0.0)
   by 0x41B3E0: DropDown::~DropDown() (dropdown.cpp:96)
   by 0x442F91: GCContainer::~GCContainer() (gccontainer.cpp:34)
   by 0x4995E9: Window::~Window() (window.cpp:157)
   by 0x469E20: 

Bug#429245: libguichan0: Segfaults when destroying dropdown widgets

[Guillaume Melquiond]

Package: libguichan0
Version: 0.6.1-2
Severity: important
Tags: patch

In DropDown::~DropDown, listener are removed from objets, after these
objects have already been destroyed. It induces a segfault at the
startup of The Mana World when it tries to delete the server selection
dropbox. The attached patch fixes it by removing the listeners before
deleting the objects.


-- System Information:
Debian Release: lenny/sid
  APT prefers testing
  APT policy: (990, 'testing'), (500, 'unstable')
Architecture: amd64 (x86_64)

Kernel: Linux 2.6.18-4-amd64 (SMP w/2 CPU cores)
Locale: LANG=fr_FR.UTF-8, LC_CTYPE=fr_FR.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

Versions of packages libguichan0 depends on:
ii  libc6   2.5-11   GNU C Library: Shared
libraries
ii  libgcc1 1:4.2-20070528-1 GCC support library
ii  libstdc++6  4.2-20070528-1   The GNU Standard C++
Library v3

libguichan0 recommends no packages.

-- no debconf information

--- src/widgets/dropdown.cpp.old	2007-06-16 15:36:56.0 +0200
+++ src/widgets/dropdown.cpp	2007-06-16 15:37:23.0 +0200
@@ -123,24 +123,24 @@
 
 DropDown::~DropDown()
 {
-if (mInternalScrollArea)
+if (widgetExists(mListBox))
 {
-delete mScrollArea;
+mListBox-removeActionListener(this);
 }
 
-if (mInternalListBox)
+if (mScrollArea != NULL)
 {
-delete mListBox;
+mScrollArea-removeDeathListener(this);
 }
 
-if (widgetExists(mListBox))
+if (mInternalScrollArea)
 {
-mListBox-removeActionListener(this);
+delete mScrollArea;
 }
 
-if (mScrollArea != NULL)
+if (mInternalListBox)
 {
-mScrollArea-removeDeathListener(this);
+delete mListBox;
 }
 }
 

Bug#429245: libguichan0: Segfaults when destroying dropdown widgets

[Guillaume Melquiond]

I have hit another segfault while destroying dropdown widgets. This
time, it is due to the internal focus handler being automatically
destroyed by DropDown::~DropDrow yet still used later by the inherited
destructors. So I attach a new patch. In addition to the bugfix
contained in the previous patch, it removes the internal focus handler
while it is still alive.
--- src/widgets/dropdown.cpp.old	2007-06-16 15:36:56.0 +0200
+++ src/widgets/dropdown.cpp	2007-06-16 17:02:50.0 +0200
@@ -123,25 +123,27 @@
 
 DropDown::~DropDown()
 {
-if (mInternalScrollArea)
+if (widgetExists(mListBox))
 {
-delete mScrollArea;
+mListBox-removeActionListener(this);
 }
 
-if (mInternalListBox)
+if (mScrollArea != NULL)
 {
-delete mListBox;
+mScrollArea-removeDeathListener(this);
 }
 
-if (widgetExists(mListBox))
+if (mInternalScrollArea)
 {
-mListBox-removeActionListener(this);
+delete mScrollArea;
 }
 
-if (mScrollArea != NULL)
+if (mInternalListBox)
 {
-mScrollArea-removeDeathListener(this);
+delete mListBox;
 }
+
+setInternalFocusHandler(NULL);
 }
 
 void DropDown::draw(Graphics* graphics)