GdkEvent copy under signal handler (was: Fixed memory leaks in gdk-1.2.defs)

[Kevin Ryde <user42@zip.com.au>]

[-- Attachment #1: Type: text/plain, Size: 1559 bytes --]

Marko Rauhamaa <marko@pacujo.net> writes:
>
>  - gdk-1.2.defs: Removed copy options (previously: gdk_..._ref,
>    gtk_no_copy, gtk_fake_copy, gdk_..._copy).

This included GdkEvent I take it.  It's best to give the full name of
each function and/or type, rather than "...", so they can be located
by a grep etc.

I believe this has introduced a bug in the handling of GdkEvent
objects.  An event passed to a signal handler is not copied, and can
end up being freed while still in use.

To observe this, add a fake version of gdk_event_free into guile-gtk.c

	void
	gdk_event_free (GdkEvent *e)
	{
	  printf ("gdk_event_free %p\n", e);
	}

And run a little program

	(use-modules (gtk gtk))
	(define w (gtk-window-new 'toplevel))
	(gtk-widget-add-events w '(pointer-motion-mask))
	(define e #f)
	(gtk-signal-connect w "motion_notify_event"
	                    (lambda (event)
	                      (format #t "prev event ~a\n" e)
	                      (set! e event)))
	(gtk-widget-show-all w)
	(gtk-main)

move the mouse across the window created.  The output is something
like

	gdk_event_free 0x80b1b7c
	prev event #<GdkEvent 80b1b7c>

0x80b1b7c has been freed while it's still in the "e" variable.


I propose to add back the copy option,

        * gdk-1.2.defs (GdkEvent): Reinstate copy option gdk_event_copy, and
        describe why it's there.

I believe all functions which return a GdkEvent (namely gdk_event_get,
gdk_event_peek, gdk_event_get_graphics_expose) have their own
copyingness specified and are therefore unaffected by this change.


[-- Attachment #2: gdk-1.2.defs.gdk-event-copy.diff --]
[-- Type: text/plain, Size: 714 bytes --]

--- gdk-1.2.defs.~1.32.~	2003-06-15 09:28:33.000000000 +1000
+++ gdk-1.2.defs	2003-06-15 15:33:29.000000000 +1000
@@ -407,7 +407,16 @@
 (define-type-alias GdkBitmap GdkWindow)
 (define-type-alias GdkDrawable GdkWindow)
 
+;; A GdkEvent passed to a signal handler is freed when the handler returns.
+;; The copy option here ensures the boxed object we create from it has an
+;; unlimited lifespan.
+;;
+;; For reference, the Gdk internal gdk_event_dispatch, which is a
+;; GSourceFunc in the main loop, is where the freeing happens.  An event is
+;; dequeued, the handler function called, and the event freed.
+;;
 (define-boxed GdkEvent
+  (copy gdk_event_copy)
   (free gdk_event_free))
 
 (define-boxed GdkColor