[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[pygame] [BUG] pygame.scrap crashes on X.Org



Hi,

the pygame.scrap module has several issues, which prevent it from being
usable. The following list applies to X.Org 6.9 (tested on FreeBSD
RELENG_6 only). The mentioned bugs are fixed in the attached patch.

* No SDL checks. The pygame modules usually throw an error on missing
  initialization. scrap does not, but crashes upon using its methods
  directly (all platforms (?)).
* Xlib throws XConvertSelection errors upon trying to receive the
  XA_PRIMARY selection, if a different window is selected. This is
  fixed by checking for any possible selection buffer in XA_PRIMARY,
  XA_SECONDARY, XA_CUT_BUFFER0 and using the returned owner
  window. (X.Org only.)
* Dead lock upon receiving a correct selection buffer. The
  SelectionNotify event does not seem to be passed to the pygame window,
  which could cause it to end up in an endless loop. (X.Org only.)
  Fixed by removing the response loop.

Please recheck and test the patch before commiting.

Regards
Marcus
Index: src/pygame.h
===================================================================
--- src/pygame.h	(Revision 881)
+++ src/pygame.h	(Arbeitskopie)
@@ -86,10 +86,12 @@
 #define JOYSTICK_INIT_CHECK() \
 	if(!SDL_WasInit(SDL_INIT_JOYSTICK)) \
 		return RAISE(PyExc_SDLError, "joystick system not initialized")
+#define SCRAP_INIT_CHECK() \
+    if(!scrap_initialized()) \
+        return RAISE(PyExc_SDLError, "scrap system not initialized")
 
 
 
-
 /* BASE */
 #define PYGAMEAPI_BASE_FIRSTSLOT 0
 #define PYGAMEAPI_BASE_NUMSLOTS 13
Index: src/scrap.c
===================================================================
--- src/scrap.c	(Revision 881)
+++ src/scrap.c	(Arbeitskopie)
@@ -34,8 +34,6 @@
 #include "pygame.h"
 #include "pygamedocs.h"
 
-
-
 /* Miscellaneous defines */
 #define PUBLIC
 #define PRIVATE	static
@@ -53,6 +51,11 @@
     #error Unknown window manager for clipboard handling
 #endif /* scrap type */
 
+/**
+ * Indicates, whether pygame.scrap was initialized or not.
+ */
+static int _scrapinitialized = 0;
+
 /* MAC_SCRAP delegates all functionality, we just need a small stub */
 #if !defined(MAC_SCRAP)
 
@@ -284,9 +287,60 @@
 #if defined(X11_SCRAP)
 /* The system message filter function -- handle clipboard messages */
 PRIVATE int clipboard_filter(const SDL_Event *event);
-#endif
 
+/**
+ * Tries to determine the X window with a valid selection.
+ * Default is to check
+ *
+ * - passed parameter
+ * - XA_PRIMARY
+ * - XA_SECONDARY
+ * - XA_CUT_BUFFER0
+ * 
+ * in this order.
+ */
+PRIVATE Window
+get_scrap_owner (Atom *selection)
+{
+    Window owner = XGetSelectionOwner (SDL_Display, *selection);
+    if (owner != None)
+        return owner;
+
+    owner = XGetSelectionOwner (SDL_Display, XA_PRIMARY);
+    if (owner != None)
+    {
+        *selection = XA_PRIMARY;
+        return owner;
+    }
+
+    owner = XGetSelectionOwner (SDL_Display, XA_SECONDARY);
+    if (owner != None)
+    {
+        *selection = XA_SECONDARY;
+        return owner;
+    }
+
+    owner = XGetSelectionOwner (SDL_Display, XA_CUT_BUFFER0);
+    if (owner != None)
+    {
+        *selection = XA_CUT_BUFFER0;
+        return owner;
+    }
+    return None;
+}
+
+#endif /* X11_SCRAP */
+
+/**
+ * Indicates, whether the pygame.scrap module was successfully initialized.
+ */
 PUBLIC int
+scrap_initialized (void)
+{
+    return _scrapinitialized;
+}
+
+PUBLIC int
 init_scrap(void)
 {
   SDL_SysWMinfo info;
@@ -332,6 +386,10 @@
 
 #endif /* scrap type */
     }
+
+  if (!retval)
+      _scrapinitialized = 1; /* Initialization successful. */
+
   return(retval);
 }
 
@@ -488,27 +546,37 @@
       }
     else
       {
-        int selection_response = 0;
-        SDL_Event event;
+        Atom source = XA_PRIMARY;
 
         owner = SDL_Window;
         Lock_Display();
-        selection = XInternAtom(SDL_Display, "SDL_SELECTION", False);
-        XConvertSelection(SDL_Display, XA_PRIMARY, format,
-                                        selection, owner, CurrentTime);
+
+        /* We will try some default selections before we are going
+         * to fail. */
+        owner = get_scrap_owner (&source);
+        selection = XInternAtom (SDL_Display, "SDL_SELECTION", False);
+        XConvertSelection(SDL_Display, source, format, selection, owner,
+                          CurrentTime);
         Unlock_Display();
-        while ( ! selection_response )
-          {
-            SDL_WaitEvent(&event);
-            if ( event.type == SDL_SYSWMEVENT )
-              {
-                XEvent xevent = event.syswm.msg->event.xevent;
 
-                if ( (xevent.type == SelectionNotify) &&
-                     (xevent.xselection.requestor == owner) )
-                    selection_response = 1;
-              }
-          }
+/* This loop caused a dead lock, once the Xlib crashes were fixed.
+ * It does not seem to be necessary anyways. */
+
+/*        int selection_response = 0; */
+/*        SDL_Event event; */
+/*         while ( ! selection_response ) */
+/*           { */
+/*             SDL_WaitEvent(&event); */
+/*             if ( event.type == SDL_SYSWMEVENT ) */
+/*               { */
+/*                 XEvent xevent = event.syswm.msg->event.xevent; */
+
+/*                 if ( (xevent.type == SelectionNotify) && */
+/*                      (xevent.xselection.requestor == owner) ) */
+/*                     selection_response = 1; */
+/*               } */
+/*           } */
+
       }
     Lock_Display();
     if ( XGetWindowProperty(SDL_Display, owner, selection, 0, INT_MAX/4,
@@ -717,6 +785,8 @@
 
 	int scrap_type;
 
+        SCRAP_INIT_CHECK ();
+        
 	if(!PyArg_ParseTuple(args, "i", &scrap_type))
 		return NULL;
 
@@ -754,6 +824,8 @@
     char *scrap = NULL;
     int scrap_type;
 
+    SCRAP_INIT_CHECK ();
+
     //if(!PyArg_ParseTuple(args, "is#", &scrap_type, &scrap, &scraplen))
     if(!PyArg_ParseTuple(args, "it#", &scrap_type, &scrap, &scraplen))
         return NULL;
@@ -772,6 +844,8 @@
 
 
 static PyObject* scrap_lost_scrap(PyObject* self, PyObject* args) {
+    SCRAP_INIT_CHECK ();
+
     if ( lost_scrap() ) {
         return PyInt_FromLong(1);
     } else {
@@ -788,6 +862,9 @@
 	static PyObject *mac_scrap_module = NULL;
 	PyObject *method;
 	PyObject *result;
+
+        SCRAP_INIT_CHECK ();
+
 	if (mac_scrap_module == NULL) {
 		mac_scrap_module = PyImport_ImportModule("pygame.mac_scrap");
 		if (mac_scrap_module == NULL) {
Index: src/scrap.h
===================================================================
--- src/scrap.h	(Revision 881)
+++ src/scrap.h	(Arbeitskopie)
@@ -24,6 +24,11 @@
 /* Miscellaneous defines */
 #define T(A, B, C, D)	(int)((A<<24)|(B<<16)|(C<<8)|(D<<0))
 
+/**
+ * Indicates, whether the pygame.scrap module was successfully initialized.
+ */
+extern int scrap_initialized (void);
+
 extern int init_scrap(void);
 extern int lost_scrap(void);
 extern void put_scrap(int type, int srclen, char *src);

Attachment: pgpOxM7clAUCE.pgp
Description: PGP signature