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

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);