PATCH: Re: GTKMM Clipboard::wait_for_targets() bug?

[Peter Clifton <pcjc2 cam ac uk>]

On Wed, 2009-06-17 at 14:23 +0100, Peter Clifton wrote:
> Hi,
> 
> I'm playing with getting inkscape to paste successfully from our
> application (gschem), and have encountered some weird corruption-like
> problems with inkscape not successfully getting the correct list of
> targets on the clipboard.
> 
> Running with valgrind, I see the following:
> 
> ==28334== Invalid read of size 1
> ==28334==    at 0x40276E8: strlen (mc_replace_strmem.c:242)
> ==28334==    by 0x58F888A: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (in /usr/lib/libstdc++.so.6.0.10)
> ==28334==    by 0x451B74A: Glib::ustring::ustring(char const*) (in /usr/lib/libglibmm-2.4.so.1.2.0)
> ==28334==    by 0x8389DF3: Inkscape::UI::ClipboardManagerImpl::_getBestTarget() (containerhandle_shared.h:291)
> ==28334==    by 0x838AE7D: Inkscape::UI::ClipboardManagerImpl::paste(bool) (clipboard.cpp:305)
> ==28334==    by 0x80C38C3: sp_selection_paste(SPDesktop*, bool) (selection-chemistry.cpp:954)
> ==28334==    by 0x824642C: sp_action_perform(SPAction*, void*) (action.cpp:181)
> ==28334==    by 0x80D41E7: sp_shortcut_invoke(unsigned int, Inkscape::UI::View::View*) (shortcuts.cpp:55)
> ==28334==    by 0x824CB36: on_window_key_press(_GdkEventKey*) (window.cpp:34)
> ==28334==    by 0x42AFEF1: (anonymous namespace)::Widget_signal_key_press_event_callback(_GtkWidget*, _GdkEventKey*, void*) (slot.h:515)
> ==28334==    by 0x47EE525: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:84)
> ==28334==    by 0x4D3BC7A: g_closure_invoke (gclosure.c:767)
> 
> The string which causes the invalid read is the target returned from
> inkscape's Inkscape::UI::ClipboardManagerImpl::_get_best_target()
> function. The string originates from a call to
> Gtk::Clipboard::wait_for_targets() within the _get_best_target()
> function.
> 
> ==28334==  Address 0x7e6164c is 12 bytes inside a block of size 22 free'd
> ==28334==    at 0x402599A: operator delete(void*) (vg_replace_malloc.c:342)
> ==28334==    by 0x58F79FC: std::string::_Rep::_M_destroy(std::allocator<char> const&) (in /usr/lib/libstdc++.so.6.0.10)
> ==28334==    by 0x451E5C0: Glib::ustring::~ustring() (in /usr/lib/libglibmm-2.4.so.1.2.0)
> ==28334==    by 0x41F6206: Gtk::Clipboard::wait_for_targets() const (new_allocator.h:118)
> ==28334==    by 0x8389DBD: Inkscape::UI::ClipboardManagerImpl::_getBestTarget() (clipboard.cpp:1242)
> ==28334==    by 0x838AE7D: Inkscape::UI::ClipboardManagerImpl::paste(bool) (clipboard.cpp:305)
> ==28334==    by 0x80C38C3: sp_selection_paste(SPDesktop*, bool) (selection-chemistry.cpp:954)
> ==28334==    by 0x824642C: sp_action_perform(SPAction*, void*) (action.cpp:181)
> ==28334==    by 0x80D41E7: sp_shortcut_invoke(unsigned int, Inkscape::UI::View::View*) (shortcuts.cpp:55)
> ==28334==    by 0x824CB36: on_window_key_press(_GdkEventKey*) (window.cpp:34)
> ==28334==    by 0x42AFEF1: (anonymous namespace)::Widget_signal_key_press_event_callback(_GtkWidget*, _GdkEventKey*, void*) (slot.h:515)
> ==28334==    by 0x47EE525: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:84)
> 
> 
> What I can't understand, is why the destructor for the ustring is being
> called in Gtk::Clipboard::wait_for_targets(), since that ustring should
> be "owned" by the std::list of ustrings which is being returned.
> 
> I'm not a C++ programmer by any means, so any help in finding the right
> path to pursue in debugging this would be much appreciated.



It seems that the implicit creation of a Glib::StringArrayHandle upon
return was causing the problem. The constructor which takes a container
(our std::list of ustring targets), does not associate ownership of the
array elements with the newly created Glib::StringArrayHandle, it just
stores pointers.

When our "listTargets" std::list goes out of scope, its elemensts are
free'd, thus causing the corruption / problems when our caller tries to
use its result.

I'm no C++ programmer, so the best I could come up with was using the
alternative array based constructor for Glib::StringArrayHandle, passing
an array of char *, and associating their ownership with the returned
list.

I _think_ this works in terms of non-leakiness, but I'm not sure. Anyway
- the fact it fixes the bug ought to help identify a better fix if
necessary.

Could someone point me to the appropriate place / person to file a bug /
bug regarding this fix?

Best regards,

-- 
Peter Clifton

Electrical Engineering Division,
Engineering Department,
University of Cambridge,
9, JJ Thomson Avenue,
Cambridge
CB3 0FA

Tel: +44 (0)7729 980173 - (No signal in the lab!)