From ripley at stats.ox.ac.uk  Mon Sep  1 12:06:42 2008
From: ripley at stats.ox.ac.uk (Prof Brian Ripley)
Date: Mon, 1 Sep 2008 11:06:42 +0100 (BST)
Subject: [Rd] (PR#12628) Second X11 call with invalid display crashes R
 after first X11 call.
In-Reply-To: <Pine.GSO.4.56.0808281359120.16731@devsun3201.insightful.com>
References: <20080828085510.0B8F1282C765@mail.pubhealth.ku.dk>
	<alpine.LFD.1.10.0808281102020.15220@auk.stats.ox.ac.uk>
	<Pine.GSO.4.56.0808281359120.16731@devsun3201.insightful.com>
Message-ID: <alpine.LFD.1.10.0809011021550.9719@gannet.stats.ox.ac.uk>

This is a different issue, and one that I am unable to reproduce.  If I 
start PUTTY with X11 forwarding and no X server, I get the usual message 
from X11().

As I understand it, when running available.packages() you are getting a 
call to the tcltk version of chooseCRANmirror().  Again that works 
correctly for me, falling back to a text menu.  This is related to whether 
capabilities("X11") returns TRUE for X11, and that does attempt to open a 
display via in_R_X11_access.

I think this has to be something specific to Bill's X setup.  Note that we 
don't even always set X11 handlers (Rf_setX11Display makes it optional), 
so I don't really want to follow your suggestion and set a handler 
unconditionally.  If we were to go that route, I think setting a separate 
simple handler before the calls to XOpenDisplay in X11_Open and 
in_R_X11_access would be more appropriate.  I'll send Bill a version that 
does that for testing, as I cannot reproduce this.

Rf_setX11Display is always called with setHandlers=TRUE from R itself. 
This seems lost in the mists of time (from the original version of 
src/modules/X11/devX11.h in 2001) so unless anyone knows of a package that 
uses it perhaps we should remove the option.

On Thu, 28 Aug 2008, Bill Dunlap wrote:

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

-- 
Brian D. Ripley,                  ripley at stats.ox.ac.uk
Professor of Applied Statistics,  http://www.stats.ox.ac.uk/~ripley/
University of Oxford,             Tel:  +44 1865 272861 (self)
1 South Parks Road,                     +44 1865 272866 (PA)
Oxford OX1 3TG, UK                Fax:  +44 1865 272595



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