FP_LOADER_THREAD segfault

[Chris Pavlina]

The footprint loader thread is occasionally segfaulting, see attached.

[Chris Pavlina (pavlina-chris)]

I'll get to this tomorrow or so. Pending a reply from nickoe with some more information from his core dump.

[Chris Pavlina (pavlina-chris)]

(gdb) bt
#0 0x0000007200000063 in ()
#1 0x00007f7bc496ae1b in FP_LOADER_THREAD::ProcessEntry(FOOTPRINT_PREVIEW_PANEL::CACHE_ENTRY&) (this=0xad41130, aEntry=...)
    at /home/nickoe/kicad-git/pcbnew/footprint_preview_panel.cpp:181
#2 0x00007f7bc496af2b in FP_LOADER_THREAD::Entry() (this=0xad41130) at /home/nickoe/kicad-git/pcbnew/footprint_preview_panel.cpp:193
#3 0x00007f7be22b4492 in wxThread::CallEntry() () at /usr/lib/libwx_baseu-3.0.so.0
#4 0x00007f7be22bd240 in () at /usr/lib/libwx_baseu-3.0.so.0
#5 0x00007f7bdf4142e7 in start_thread () at /usr/lib/libpthread.so.0
#6 0x00007f7bdf15554f in clone () at /usr/lib/libc.so.6
(gdb) f 1
#1 0x00007f7bc496ae1b in FP_LOADER_THREAD::ProcessEntry (this=0xad41130, aEntry=...) at /home/nickoe/kicad-git/pcbnew/footprint_preview_panel.cpp:181
181 handler->QueueEvent( new wxCommandEvent( wxEVT_COMMAND_TEXT_UPDATED, 1 ) );
(gdb) p handler
$1 = (wxEvtHandler *) 0xba896c0
(gdb) p m_parent
$2 = (FOOTPRINT_PREVIEW_PANEL *) 0xba896c0
(gdb)

[Nick Østergaard (nickoe)]

I can add the following:

(gdb) bt
#0 0x0000007200000063 in ()
#1 0x00007f7bc496ae1b in FP_LOADER_THREAD::ProcessEntry(FOOTPRINT_PREVIEW_PANEL::CACHE_ENTRY&) (this=0xad41130, aEntry=...)
    at /home/nickoe/kicad-git/pcbnew/footprint_preview_panel.cpp:181
#2 0x00007f7bc496af2b in FP_LOADER_THREAD::Entry() (this=0xad41130) at /home/nickoe/kicad-git/pcbnew/footprint_preview_panel.cpp:193
#3 0x00007f7be22b4492 in wxThread::CallEntry() () at /usr/lib/libwx_baseu-3.0.so.0
#4 0x00007f7be22bd240 in () at /usr/lib/libwx_baseu-3.0.so.0
#5 0x00007f7bdf4142e7 in start_thread () at /usr/lib/libpthread.so.0
#6 0x00007f7bdf15554f in clone () at /usr/lib/libc.so.6
(gdb) f 1
#1 0x00007f7bc496ae1b in FP_LOADER_THREAD::ProcessEntry (this=0xad41130, aEntry=...) at /home/nickoe/kicad-git/pcbnew/footprint_preview_panel.cpp:181
181 handler->QueueEvent( new wxCommandEvent( wxEVT_COMMAND_TEXT_UPDATED, 1 ) );
(gdb) p handler
$1 = (wxEvtHandler *) 0xba896c0
(gdb) p m_parent
$2 = (FOOTPRINT_PREVIEW_PANEL *) 0xba896c0
(gdb)

[Chris Pavlina (pavlina-chris)]

Finally got this segfault myself, with symbols enabled in wx...backtrace is bizarre.

(gdb) bt
#0 0x00007fc9adecef24 in wxGenericImageList::RemoveAll() (this=this@entry=0x69c3c00) at ./src/generic/imaglist.cpp:234
#1 0x00007fc9adecef67 in wxGenericImageList::~wxGenericImageList() (this=0x69c3c00, __in_chrg=<optimized out>) at ./src/generic/imaglist.cpp:40
#2 0x00007fc994a2f0cf in FP_LOADER_THREAD::ProcessEntry(FOOTPRINT_PREVIEW_PANEL::CACHE_ENTRY&) (this=0x86c6ca0, aEntry=...) at /home/cmp/git/kicad/pcbnew/footprint_preview_panel.cpp:181
#3 0x00007fc994a2f1df in FP_LOADER_THREAD::Entry() (this=0x86c6ca0) at /home/cmp/git/kicad/pcbnew/footprint_preview_panel.cpp:193
#4 0x00007fc9ad763492 in wxThread::CallEntry() (this=this@entry=0x86c6ca0) at ./include/wx/thrimpl.cpp:356
#5 0x00007fc9ad76c240 in wxThreadInternal::PthreadStart(wxThread*) (thread=0x86c6ca0) at ./src/unix/threadpsx.cpp:867
#6 0x00007fc9aa9842e7 in start_thread () at /usr/lib/libpthread.so.0
#7 0x00007fc9aa6c554f in clone () at /usr/lib/libc.so.6

[Chris Pavlina (pavlina-chris)]

Not sure if relevant, but the footprint being queried at the time was invalid:

(gdb) f 2
(gdb) p aEntry
$9 = (FP_LOADER_THREAD::CACHE_ENTRY &) @0x7fc98d72b958: {fpid = {
    nickname = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "", <No data fields>},
    item_name = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "footprint", <No data fields>},
    revision = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "", <No data fields>}}, module = 0x0,
  status = FPS_NOT_FOUND}

(gdb) p *m_iface
$10 = {m_loaderQueue = std::deque with 0 elements, m_cachedFootprints = std::map with 2 elements = {[{
      nickname = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "", <No data fields>},
      item_name = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "footprint", <No data fields>},
      revision = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "", <No data fields>}}] = {fpid = {
        nickname = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "", <No data fields>},
        item_name = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "footprint", <No data fields>},
        revision = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "", <No data fields>}}, module = 0x0,
      status = FPS_NOT_FOUND}, [{
      nickname = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "IPC7351-Nominal", <No data fields>},
      item_name = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "CAPC1608X55", <No data fields>},
      revision = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "", <No data fields>}}] = {fpid = {
        nickname = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "IPC7351-Nominal", <No data fields>},
        item_name = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "CAPC1608X55", <No data fields>},
        revision = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "", <No data fields>}}, module = 0x7fc985cf2660,
      status = FPS_READY}}, m_current_fp = {
    nickname = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "", <No data fields>},
    item_name = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "footprint", <No data fields>},
    revision = {<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >> = "", <No data fields>}}, m_lock = {m_mutex = {m_mut = {
        __data = {__lock = 0, __count = 0, __owner = 0, __nusers = 0, __kind = 128, __spins = 0, __elision = 0, __list = {__prev = 0x0, __next = 0x0}},
        __size = '\000' <repeats 16 times>, "\200", '\000' <repeats 22 times>, __align = 0}}}}

[Chris Pavlina (pavlina-chris)]

Ooh, I have a hunch. I got this segfault as the window was being closed. I suspect footprint_preview_panel.cpp:178 (GetEventHandler()) got an invalid object with a corrupt/nonsense vtable, sending it off to wxGenericImageList::~wxGenericImageList() instead of QueueEvent().

Proposed fix now is to make sure FOOTPRINT_PREVIEW_PANEL properly joins/stops the thread. This may be related to the warning on the waitMode parameter in http://docs.wxwidgets.org/3.1.0/classwx_thread.html#a40ef94232664185b4f1f8693eebeba86

[Chris Pavlina (pavlina-chris)]

This is complicated by the fact that the threads were made detachable *entirely because* they kept running downloads in the background and prevented the dialog from closing. Perhaps the event bit needs to be pulled into the synchronized interface class, with some magic to just disable posting events when the dialog is destroyed. We can't go back to minute-long freezes on close.

[KiCad Janitor (kicad-janitor)]

Fixed in revision 61c2beea22edc7cea9d09cf3522ee633bcb79f3e
https://git.launchpad.net/kicad/patch/?id=61c2beea22edc7cea9d09cf3522ee633bcb79f3e