GNOME Bugzilla – Bug 586626
Clipboard::wait_for_targets() returns list pointing to free'd memory
Last modified: 2009-09-16 15:23:09 UTC
Steps to reproduce: Noted in inkscape, under valgrind. Stack trace: Other information: > It seems that the implicit creation of a Glib::StringArrayHandle upon > return is 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.
Created attachment 137165 [details] [review] Proposed patch fixing the bug
Yes, the patch is probably appropriate. CCing Daniel Elstner though, because he probably has an opinion about this.
Confirmed! I ran into the same bug, independently from Peter, but also in Inkscape. I haven't tested his patch, but I hacked an ugly workaround into Inkscape to circumvent this memory corruption. Obviously, I would love to see this fixed in gtkmm in the end. TIA, Diederik
Forgot to mention that more details can be found here: https://bugs.launchpad.net/inkscape/+bug/296778/comments/13 and here: http://mail.gnome.org/archives/gtk-devel-list/2009-June/msg00062.html
Sorry for the delay, I completely missed this bug report. Yes, the code is horribly wrong. The patch is mostly correct. Of course, the real fix would be to use ArrayHandle<> with custom Traits to define the GdkAtom -> gchar* conversion, but that's not possible at this point as it would break ABI. I applied a modified version of the patch with a memory leak plugged and some stylistic changes. There was another similar misuse of the array handle in that source file, which I fixed, too. commit 571dd7a71fadb3d61d698c84bc8d5f0a980c1af7 Author: Daniel Elstner <danielk@openismus.com> Date: Tue Sep 15 16:49:51 2009 +0200 Correct misuse of container handle (bgo #586626) * gtk/src/clipboard.{ccg,hg} (Clipboard::wait_for_targets): Create an intermediate array of allocated C strings and transfer ownership on return, instead of trying to returned a handle to a container with local scope. (Based on patch by Peter Clifton.) (SignalProxy_TargetsReceived_gtk_callback): Correct another misuse of the container handles similar to the above, except for the crash. (SignalProxy_RichTextReceived_gtk_callback): Simplify the code and plug a memory leak in the event of an exception.
Thanks, Daniel. Could you backport this to gtkmm 2.16 too, please, if that makes sense.
I cherry-picked the commit onto gtkmm-2-16, it applied flawlessly.