[Rd] (PR#12628) Second X11 call with invalid display crashes R after first X11 call.

[Bill Dunlap]

On Thu, 28 Aug 2008, Prof Brian Ripley wrote:

> This is an Xt error, whereas the first one is an Xlib error.  It is easy
> to trap it.
>
> You do realize that this will never work?  In the current setup all open
> X11 devices must be on the same display, and 'display' is ignored if a
> device is already open.  (I don't know if that is documented: I did not
> write the Xlib device nor the help page.)

I have a different problem with the x11() bringing
down R when $DISPLAY does not refer to a valid X
server.  I typically sit at a Windows laptop running
a Cygwin X server and use the ssh client/terminal
emulator program 'putty' to run a shell on on Linux,
where I run R.  Putty tunnels X protocol and sets
DISPLAY (to something like localhost:11.0) whether or
not the PC's X server is running.  If I have forgotten
to start the PC's X server R currently shuts down when
I try to start x11() or the tcl/tk stuff that
available.packages() runs.  E.g.,
  > x11()
  X connection to localhost:11.0 broken (explicit kill or server shutdown).
  linuxPrompt%

If I call the XIOErrorHandler(R_X11IOErr) before calling XOpenDisplay()
(in src/modules/X11/devX11.c) then I can trap that error
and have x11() throw an ordinary R error:
  > x11()
  Error in x11() : X11 I/O error while opening device "localhost:11.0"
  >

The following patch sets the IOErrorHandler before XOpenDisplay
and changes the handler a bit so it doesn't assume that some
global variables have been set yet.

It doesn't help the problem in available.packages().  That must
make its own unprotected call to XOpenDisplay().

Index: src/modules/X11/devX11.c
===================================================================
--- src/modules/X11/devX11.c	(revision 46454)
+++ src/modules/X11/devX11.c	(working copy)
@@ -1080,19 +1080,30 @@

 static int R_X11IOErr(Display *dsp)
 {
-    int fd = ConnectionNumber(display);
-    /*
-    while (nfonts--)  R_XFreeFont(display, fontcache[nfonts].font);
-    nfonts = 0;
-    */
-    removeInputHandler(&R_InputHandlers,
-		       getInputHandler(R_InputHandlers,fd));
-    /*
-    XCloseDisplay(display);
-    displayOpen = FALSE;
-    strcpy(dspname, "");
-    */
-    error(_("X11 fatal IO error: please save work and shut down R"));
+    int fd ;
+    /* display is only set after successful call to XOpenDisplay.
+     * If it is non-null I think it should be the same as dsp, but
+     * display==NULL & dsp!=NULL means that we have run into a problem
+     * while executing XOpenDisplay.
+     */
+    if (display && displayOpen) {
+       fd = ConnectionNumber(display);
+       /*
+       while (nfonts--)  R_XFreeFont(display, fontcache[nfonts].font);
+       nfonts = 0;
+       */
+       removeInputHandler(&R_InputHandlers,
+		          getInputHandler(R_InputHandlers,fd));
+       /*
+       XCloseDisplay(display);
+       displayOpen = FALSE;
+       strcpy(dspname, "");
+       */
+       error(_("X11 fatal IO error: please save work and shut down R"));
+    } else if (dsp) { /* In XOpenDisplay() is the intent */
+       char *devname =  XDisplayName(dspname) ;
+       error(_("X11 I/O error while opening device \"%s\""), devname);
+    }
     return 0; /* but should never get here */
 }

@@ -1247,15 +1258,21 @@
     /* initialize the X11 device driver data structures. */

     if (!displayOpen) {
+        /* Set IOErrorHandler before trying to open the display
+         * just in case DISPLAY is putty tunnelling X11 protocol
+         * to a machine that is not runing an X server.  Set dspname
+         * now so R_X11IOErr can show it to user. */
+	XSetIOErrorHandler(R_X11IOErr);
+	strncpy(dspname, p, 101);
+	dspname[100] = '\0';
 	if ((display = XOpenDisplay(p)) == NULL) {
+            dspname[0] = '\0' ;
 	    warning(_("unable to open connection to X11 display '%s'"), p);
 	    return FALSE;
 	}
 	DisplayOpened = TRUE;
 	Rf_setX11Display(display, gamma_fac, colormodel, maxcube, TRUE);
 	displayOpen = TRUE;
-	strncpy(dspname, p, 101);
-	dspname[100] = '\0';
 	if(xd->handleOwnEvents == FALSE)
 	    addInputHandler(R_InputHandlers, ConnectionNumber(display),
 			    R_ProcessX11Events, XActivity);