Fandom

Sawfish

Xinerama and X ConfigureWindow fix

773pages on
this wiki
Add New Page
Talk0 Share

Ad blocker interference detected!


Wikia is a free-to-use site that makes money from advertising. We have a modified experience for viewers using ad blockers

Wikia is not accessible if you’ve made further modifications. Remove the custom ad blocker rule(s) and the page will load as expected.

Browse all patches

Author Edit

Janek Kozicki

Synopsis Edit

This bug manifests itself when using xinerama (not the nvidia's twinview one but the regular OSS one). However I could not find in xinerama's source any reference to X_ConfigureWindow calls.

Problem is following: windows disappear! Seemingly random, sometimes very frequently. The processes are not killed - they are running and doing quite fine. But their window is no longer visible.

Most often it occurs with opera (dunno why), but is not limited to opera. I had it with firefox, claws-mail, and once with an xterm (I guess). Simply open a new window, and suddenly several(!) different random windows disappear. Not only the just opened one, but random others too.

I located the problem in src/windows.c remove_window() function. I tracked all calls of remove_window and saw that it is always called from src/display.c:88, an error_handler function. This error_handler receives an XErrorEvent which is then processed. So I printed to stderr the whole content of XErrorEvent like this:

static int
error_handler (Display *dpy, XErrorEvent *ev)
{
    Lisp_Window *w;

//#ifdef DEBUG
  //  print_error (ev);
//#endif

    if (ev->resourceid != 0
        && (ev->error_code == BadWindow || ev->error_code == BadDrawable))
    {
        w = x_find_window_by_id (ev->resourceid);
    
        if (w != NULL)
        {
            DB(("error_handler (%s)\n", rep_STR(w->name)));
    
            if (!WINDOW_IS_GONE_P (w))
            {
fprintf(stderr,"remove_window display.c 88\n");
fprintf(stderr,"XErrorEvent:\n");
fprintf(stderr," - type: %d\n",ev->type);
fprintf(stderr," - error_code: %d\n",ev->error_code);
fprintf(stderr," - request_code: %d\n",ev->request_code);
fprintf(stderr," - minor_code: %d\n",ev->minor_code);
fprintf(stderr," - resourceid: %d\n",ev->resourceid);
                remove_window (w, TRUE, TRUE);
            }

            /* so we call emit_pending_destroys () at some point */
            rep_mark_input_pending (ConnectionNumber (dpy));
        }
    }
        
    return 0;
}       

And then I saw, that this occurs only when following XErrorEvent is processed:

XErrorEvent:
 - type: 0
 - error_code: 3
 - request_code: 12
 - minor_code: 0

A look at xlib reference manual told me that request_code: 12 is an X_ConfigureWindow. Clearly a window that wants itself configured doesn't necessarily want to be unmapped. Perhaps it's just cleaning some stuff up.

So in the end I produced a patch shown below. I could not reproduce the bug ever since I applied it.

This bug was reported in:

Patch testing Edit

  1. Copy/paste the patch listed below into some file, eg. TEST.diff.
  2. If you don't have sawfish sources yet, have one, as described get it from GIT repo.
  3. Go into the directory where sawfish sources reside, eg. cd sawfish
  4. Test if the patch applies cleanly with this command:
    patch -p1 --ignore-whitespace --dry-run < TEST.diff
    in case of problems try also: -p0 or -p2
  5. If it applies cleanly, then remove the --dry-run from above command and run it again, otherwise ask on the mailing list.
  6. Compile sawfish: ./autogen.sh && make
  7. Install it for testing, but it depends on your linux distribution.
    1. It is always better to install sawfish as your distribution package, but it is different for each distribution.
    2. So you may try make install, which will install sawifish in /usr/local/share/sawfish/ (if you have write access). But then make sure that you run the correct version and delete it from that directory afterwards, to avoid any conflicts.
  8. Se also

PS: edit this template if you feel that those instructions can be improved.

Patch Edit

Patch against 1.5.0

--- ./sawfish-1.5.0-orig/src/display.c  2009-07-03 15:46:05.000000000 +0200
+++ ./sawfish-1.5.0/src/display.c       2009-10-23 20:41:14.000000000 +0200
@@ -84,7 +84,21 @@
            DB(("error_handler (%s)\n", rep_STR(w->name)));
            
            if (!WINDOW_IS_GONE_P (w))
-               remove_window (w, TRUE, TRUE);
+           {
+               /* don't unmap a window that had send an X_ConfigureWindow request */
+               if(
+                   /*     ev->type == 0                what is the "type" ? but I've seen that type is always 0 */
+                   /*&&*/ ev->error_code==BadWindow /* the window is bad, because it is not configured yet */
+                     &&   ev->request_code==X_ConfigureWindow
+                     &&   ev->minor_code==0 /* X_ConfigureWindow is not in an Xlib extension, so it must be 0 */
+               ) 
+               {
+                   return 0;
+               } else 
+               {
+                   remove_window (w, TRUE, TRUE);
+               }
+           }
 
            /* so we call emit_pending_destroys () at some point */
            rep_mark_input_pending (ConnectionNumber (dpy));

Patch against 1.6.0 HEAD

diff --git a/src/display.c b/src/display.c
index cbc509a..e21f35d 100644
--- a/src/display.c
+++ b/src/display.c
@@ -82,9 +82,23 @@ error_handler (Display *dpy, XErrorEvent *ev)
 	if (w != NULL)
 	{
 	    DB(("error_handler (%s)\n", rep_STR(w->name)));
-
+
 	    if (!WINDOW_IS_GONE_P (w))
-		remove_window (w, TRUE, TRUE);
+           {
+               /* don't unmap a window that had send an X_ConfigureWindow request */
+               if(
+                   /*     ev->type == 0 what is the "type" ? but I've seen that type is always 0 */
+                   /*&&*/ ev->error_code==BadWindow /* the window is bad, because it is not configured yet */
+                     &&   ev->request_code==X_ConfigureWindow
+                     &&   ev->minor_code==0 /* X_ConfigureWindow is not in an Xlib extension, so it must be 0 */
+               )
+               {
+                   return 0;
+               } else
+               {
+                   remove_window (w, TRUE, TRUE);
+               }
+           }

 	    /* so we call emit_pending_destroys () at some point */
 	    rep_mark_input_pending (ConnectionNumber (dpy));

Community's reasons for inclusion or rejection Edit

Patch submitters, please vote also! Yes, obviously your vote will be positive, but it's the place to give your explanation why this patch is good for all Sawfish users, and why it is correct - good reasons for inclusion.

When voting anonymously please write your name, so that it can be associated with your posts on the mailing list. If you are logged in you can sign yourself by typing four tilda characters: ~~~~.

  • Please vote with: Yes vote: yes., No vote: no., Try vote: let's try in experimental., Wtf vote: pondering. or Suspend wait for next release.
  • Yes vote: yes. it works! Janek Kozicki 20:07, October 23, 2009 (UTC)
  • Yes vote: yes. indeed, plus it seems to have fixed another one, which I had
I'm forced to use the FGLRX driver and in the latest versions several apps, which often redraw,
crashed randomly (especially: Geany, Iceweasel, GNOME-Terminal), it also happend when closing
several (averagely 5 or so) tabs in Iceweasel, well that still happens, but previously other apps
have then be killed, too, at least this does no longer happen now, good work! Flashrider (Christopher Bratusek) 08:43, October 24, 2009 (UTC)
  • Wtf vote: pondering. Thanks for this.  It seems to be the right idea but I think you might need to ignore some other request codes such as X_GetWindowAttributes (3) and X_GetProperty (20). There's a longer list in FvwmErrorHandler() in fvwm. Anyway, I just discovered a minimal test case X client this morning which might help. BenC
/* Based on example from http://www.linuxjournal.com/files/linuxjournal.com/linuxjournal/articles/048/4879/4879l1.html
 * Compile with "gcc -o foo foo.c -lX11" */

#include "stddef.h"
#include "X11/Xlib.h"

int main(){

  Display *dsp = XOpenDisplay( NULL );
  if( !dsp ){ return 1; }


  int screenNumber = DefaultScreen(dsp);
  unsigned long white = WhitePixel(dsp,screenNumber);
  unsigned long black = BlackPixel(dsp,screenNumber);


  Window win = XCreateSimpleWindow(dsp,
                               DefaultRootWindow(dsp),
                               50, 50,   // origin
                               200, 200, // size
                               0, black, // border
                               white );  // backgd

  XMapWindow( dsp, win );

  XDestroyWindow( dsp, win );
  XCloseDisplay( dsp );

  return 0;
}
  • Wtf vote: pondering. I can't judge, (since I don't know X, and I don't experience this issue) but Chris' comment suggests that Xinerama is not necessarily the true cause, right? Maybe better wait for Timo's comment. Thanks all! - Teika kazura 08:40, October 27, 2009 (UTC)
  • Wtf vote: pondering. There is something strange here that I don't quite understand. The error code is BadWindow, so ev->resourceid should not be a valid window id. So why does this cause Sawfish to unmap an existing window?
If an invalid sibling is specified in a ConfigureWindow request, does that cause a BadWindow error with ev->resourceid set to the window that was being configured rather than the invalid sibling? If so, are there more cases where BadWindow errors may arise so that ev->resourceid is a valid window? Tkorvola 10:08, October 27, 2009 (UTC)
Exactly - thera are more cases with BadWindow, that aren't bad after all. See comment above by BenC. The problem is that we need to identify those cases. Janek Kozicki 10:30, October 27, 2009 (UTC)
There may be more such cases, but FvwmErrorHandler is not a valid reference for them because FVWM uses a different error handling strategy: Sawfish tries to detect missing windows in the error handler but FVWM does not.
Besides, why does Sawfish execute erroneous ConfigureWindow requests in the first place? Perhaps there is something wrong in configure_request or restack_window (those would seem the only possible sources if my invalid sibling theory is correct).
It is completely unclear what any of this has to do with Xinerama, which I don't use myself. Is there any way to reproduce this error without Xinerama with reasonable probability? Tkorvola 14:32, October 27, 2009 (UTC)
I checked from the X Protocol Manual: it is the failing resource id that is returned. So on BadWindow errors, ev->resourceid should not be a valid window, and it is a mystery why it seems to be valid in this case. Tkorvola 19:06, October 27, 2009 (UTC)
  • Wtf vote: pondering. (another note) Well, unfortunately I also had just lost a window, even with this patch. I was printing some document from openoffice, clicked "print" in the print dialog. The print dialog disappeared, and with it - my rox-backdroup background image/window disappeared too. Looks like I'll be needing to run sawfish with the debug fprintf()s included in the code, and examine it's output after such bugs occur again.... It's even possible that during the weeks I lost some other window, and simply didn't notice that! Losing a background image is quite noticeable, but losing some forgotten xterm - isn't. Janek Kozicki 11:33, November 2, 2009 (UTC)
such debug logging would be much more useful if I could printf() also the command line/process name/or whatever to identify the window in question. Is it possible to find PID (and from it a command line or executable) from ev->resourceid ? Can anybody help me with that? I would be able to review logs to find what windows were closed, and check their executable's name, if they are some forgotten window, or if they indeed should be closed. Janek Kozicki 12:03, November 2, 2009 (UTC)
Hi, Janek. I think the following works. If your env is 64-bit, you may have to cast into 32-bit. - Teika kazura 06:55, November 3, 2009 (UTC)
(deleted wrong code)
Thanks I'll try this on Friday (I hope). BTW, after putting the above note, I found three other windows missing: two xterms, and one gnuplot graph plot window. Janek Kozicki 08:33, November 3, 2009 (UTC)
Sorry, I was wrong. Adding the following in your ~/.sawfish/rc make it emit the window name and pid to stderr when the window appears, but it may not cooperate well with C part hack. I'll continue what's correct for C. - Teika kazura 12:17, November 3, 2009 (UTC)
(add-hook 'add-window-hook
	  (lambda (x)
	    (format standard-error
		    "%s: %s\n" (window-name x)
		    (aref (car (cdr (cdr (get-x-property x '_NET_WM_PID))))
			  0))))
- See mailing list for C code. It almost works. - Teika kazura 12:28, November 4, 2009 (UTC)
  • Wtf vote: pondering. More testing. I will now try totally removing error_handler (I'm not sure if it will compile when I delete whole function, so I'll just make it empty):
static int
error_handler (Display *dpy, XErrorEvent *ev)
{
    return 0;
} 
Soon I will know if this works. If I encounter some 'stale' unclosed windows, then I'm sure that it's because I removed error_handler. If I don't... then I suppose I will never again see some random window disappearing. You could test this too, with firefox, etc... Janek Kozicki 07:40, November 14, 2009 (UTC)
I just want to let you know that after 3 weeks of heavy use I had no problems at all with this patch: empty error_handler. Janek Kozicki 19:09, December 4, 2009 (UTC)

Also on Fandom

Random Wiki