After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 755393 - make check test_trackable_move fails on Mac OS X
make check test_trackable_move fails on Mac OS X
Status: RESOLVED FIXED
Product: libsigc++
Classification: Bindings
Component: tests
2.6.x
Other Mac OS
: Normal normal
: ---
Assigned To: libsigc++ maintainer(s)
libsigc++ maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-09-22 08:53 UTC by Tom Schoonjans
Modified: 2015-09-26 10:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
valgrind output (18.27 KB, text/plain)
2015-09-22 08:53 UTC, Tom Schoonjans
  Details
patch: sigc::trackable: Don't move the callback list (3.94 KB, patch)
2015-09-23 06:54 UTC, Kjell Ahlstedt
committed Details | Review

Description Tom Schoonjans 2015-09-22 08:53:32 UTC
Created attachment 311833 [details]
valgrind output

When building libsigc++ 2.6.0 on Mac OS X Yosemite I noticed that make check failed with a segfault in test_trackable_move. This is the gdb backtrace:

(gdb) run
Starting program: /Library/Caches/Homebrew/libsigc++-2.6.0/tests/.libs/test_trackable_move 

Program received signal SIGSEGV, Segmentation fault.
sigc::internal::trackable_callback_list::add_callback (this=0x7, data=0x7fff5fbff018, func=0x100019240) at trackable.cc:132
132	  if (!clearing_)  // TODO: Is it okay to silently ignore attempts to add dependencies when the list is being cleared?
(gdb) bt
  • #0 sigc::internal::trackable_callback_list::add_callback
    at trackable.cc line 132
  • #1 sigc::trackable::add_destroy_notify_callback
    at trackable.cc line 74
  • #2 ??
    from /Library/Caches/Homebrew/libsigc++-2.6.0/sigc++/.libs/libsigc-2.0.0.dylib
  • #3 ??
  • #4 ??
  • #5 ??
  • #6 ??
  • #7 ??
  • #8 sigc::internal::trackable_callback_list::~trackable_callback_list



I am also attaching the valgrind output, in case it may be useful
Comment 1 Tom Schoonjans 2015-09-22 15:11:38 UTC
Here is also the output off lldb, which provides more output than gdb:

Process 76866 launched: '/Library/Caches/Homebrew/libsigc++-2.6.0/tests/.libs/test_trackable_move' (x86_64)
Process 76866 stopped
* thread #1: tid = 0x21945e0, 0x0000000100018533 libsigc-2.0.0.dylib`std::__1::list<sigc::internal::trackable_callback, std::__1::allocator<sigc::internal::trackable_callback> >::erase(std::__1::__list_const_iterator<sigc::internal::trackable_callback, void*>) [inlined] std::__1::__list_imp<sigc::internal::trackable_callback, std::__1::allocator<sigc::internal::trackable_callback> >::__unlink_nodes(__f=0x00000001002000a0, __l=0x00000001002000a0) + 34 at list:676, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x0000000100018533 libsigc-2.0.0.dylib`std::__1::list<sigc::internal::trackable_callback, std::__1::allocator<sigc::internal::trackable_callback> >::erase(std::__1::__list_const_iterator<sigc::internal::trackable_callback, void*>) [inlined] std::__1::__list_imp<sigc::internal::trackable_callback, std::__1::allocator<sigc::internal::trackable_callback> >::__unlink_nodes(__f=0x00000001002000a0, __l=0x00000001002000a0) + 34 at list:676
   673 	    _NOEXCEPT
   674 	{
   675 	    __f->__prev_->__next_ = __l->__next_;
-> 676 	    __l->__next_->__prev_ = __f->__prev_;
   677 	}
   678 	
   679 	template <class _Tp, class _Alloc>
(lldb) bt
* thread #1: tid = 0x21945e0, 0x0000000100018533 libsigc-2.0.0.dylib`std::__1::list<sigc::internal::trackable_callback, std::__1::allocator<sigc::internal::trackable_callback> >::erase(std::__1::__list_const_iterator<sigc::internal::trackable_callback, void*>) [inlined] std::__1::__list_imp<sigc::internal::trackable_callback, std::__1::allocator<sigc::internal::trackable_callback> >::__unlink_nodes(__f=0x00000001002000a0, __l=0x00000001002000a0) + 34 at list:676, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x0000000100018533 libsigc-2.0.0.dylib`std::__1::list<sigc::internal::trackable_callback, std::__1::allocator<sigc::internal::trackable_callback> >::erase(std::__1::__list_const_iterator<sigc::internal::trackable_callback, void*>) [inlined] std::__1::__list_imp<sigc::internal::trackable_callback, std::__1::allocator<sigc::internal::trackable_callback> >::__unlink_nodes(__f=0x00000001002000a0, __l=0x00000001002000a0) + 34 at list:676
    frame #1: 0x0000000100018511 libsigc-2.0.0.dylib`std::__1::list<sigc::internal::trackable_callback, std::__1::allocator<sigc::internal::trackable_callback> >::erase(this=0x0000000100200080 size=0, __p=std::__1::list<sigc::internal::trackable_callback, std::__1::allocator<sigc::internal::trackable_callback> >::const_iterator @ 0x00007fff5fbfed80) + 145 at list:1722
    frame #2: 0x000000010001759a libsigc-2.0.0.dylib`sigc::internal::trackable_callback_list::remove_callback(this=0x0000000100200080, data=0x00007fff5fbfefc8) + 538 at trackable.cc:164
    frame #3: 0x0000000100017375 libsigc-2.0.0.dylib`sigc::trackable::remove_destroy_notify_callback(this=0x00000001002000a0, data=0x00007fff5fbfefc8) const + 37 at trackable.cc:79
    frame #4: 0x0000000100019209 libsigc-2.0.0.dylib`sigc::internal::slot_rep::notify(data=0x00000001002000a0) + 105 at slot_base.cc:91
    frame #5: 0x0000000100017b36 libsigc-2.0.0.dylib`sigc::internal::trackable_callback_list::~trackable_callback_list(this=0x0000000100200060) + 438 at trackable.cc:127
    frame #6: 0x00000001000175f5 libsigc-2.0.0.dylib`sigc::internal::trackable_callback_list::~trackable_callback_list(this=0x0000000100200060) + 21 at trackable.cc:122
    frame #7: 0x0000000100017142 libsigc-2.0.0.dylib`sigc::trackable::notify_callbacks(this=0x00007fff5fbff298) + 66 at trackable.cc:85
    frame #8: 0x00000001000171f1 libsigc-2.0.0.dylib`sigc::trackable::~trackable(this=0x00007fff5fbff298) + 17 at trackable.cc:69
    frame #9: 0x0000000100001bc5 test_trackable_move`(anonymous namespace)::my_class::~my_class(this=0x00007fff5fbff298) + 21 at test_trackable_move.cc:17
    frame #10: 0x0000000100001b15 test_trackable_move`(anonymous namespace)::my_class::~my_class(this=0x00007fff5fbff298) + 21 at test_trackable_move.cc:17
    frame #11: 0x00000001000018a1 test_trackable_move`main(argc=1, argv=0x00007fff5fbff408) + 945 at test_trackable_move.cc:78
    frame #12: 0x00007fff8e6685c9 libdyld.dylib`start + 1
    frame #13: 0x00007fff8e6685c9 libdyld.dylib`start + 1
Comment 2 Kjell Ahlstedt 2015-09-22 17:43:44 UTC
I can confirm that valgrind reports "Invalid read" and "Invalid write" on my
Ubuntu Linux system. Not exactly identical to the attached output, but very
similar. It should be possible to troubleshoot on a Linux system.
Comment 3 Kjell Ahlstedt 2015-09-23 06:54:37 UTC
Created attachment 311920 [details] [review]
patch: sigc::trackable: Don't move the callback list

  class A : public sigc::trackable
  {
  public:
    void f1() { ..... }
    void f2() { ..... }
    .....
  };

  A* a1 = new A(.....);
  A* a2 = new A(.....);
  sigc::signal<void> s1;
  sigc::signal<void> s2;
  s1.connect(sigc::mem_fun(a1, &A::f1));
  s2.connect(sigc::mem_fun(a2, &A::f2));

  // If a1 is deleted here, s1 is notified, and it disconnects a1.
  // If a2 is deleted here, s2 is notified, and it disconnects a2.

  *a1 = std::move(*a2);

What will happen after a2 has been moved to a1?

Without my patch:
  When a2 is moved to a1, s1 is notified, and it disconnects a1.
  When a1 is deleted, s2 is notified, and it disconnects a2!!
  When a2 is deleted, no one is notified.

With my patch:
  When a2 is moved to a1, s1 is notified, and it disconnects a1.
                          s2 is notified, and it disconnects a2.
  When a1 is deleted, no one is notified.
  When a2 is deleted, no one is notified.

I can't think of a situation when it's reasonable to move trackable's callback
list.

It's not obvious to me why s1 shall be notified and disconnect a1 when a1 is
the destination of a move or copy assignment, but that's how copy assignment
has worked for many years, so I suppose it's the right thing to do.
Comment 4 Murray Cumming 2015-09-26 09:45:20 UTC
Thanks. Please go ahead.