OK, this is now in R-devel, but only superficially tested (b/c this is a Mac).
Please check it out.
-pd
> On 30 Apr 2019, at 23:09 , Paul Murrell <p...@stat.auckland.ac.nz> wrote:
>
> Hi Peter
>
> Yes, that looks roughly right to me. I would be in favour of your option
> (b), partly because it is probably easiest and partly because that retains
> the basic graphics device startup logic pattern that is replicated across
> all(?) graphics devices.
>
> Paul
>
> On 28/04/19 11:39 AM, peter dalgaard wrote:
>> I had a look at the current code, and AFAICT it has essentially the same
>> structure as it did back then. I think it may have finally dawned upon me
>> what the issue really is:
>> The logic is that in Rf_addX11Device, we have
>> if (!X11DeviceDriver(dev, display, width, height,
>> ps, gamma, colormodel, maxcubesize,
>> bgcolor, canvascolor, sfonts, res,
>> xpos, ypos, title, useCairo, antialias,
>> family)) {
>> free(dev);
>> errorcall(call, _("unable to start device %s"), devname);
>> }
>> dd = GEcreateDevDesc(dev);
>> GEaddDevice2(dd, devname);
>> i.e., we start the device driver, and if it fails, we throw away the "dev"
>> structure and call it a day. If it succeeds, we proceed to create a device
>> descriptor structure and add it to the list of open devices.
>> This approach means that X11DeviceDriver() cannot do anything that
>> potentially accesses the dd structure because it isn't there yet, and the
>> things it cannot do apparently includes calling R_ProcessX11Events(). [To be
>> completely sure that this is actually still true, I'd need to have a closer
>> look at what handleEvent() does.]
>> So to fix things, it would seem that you could (a) add the device before
>> attempting to start the driver, preparing to back it out if the driver fails
>> to start, or (b) add a call to R_ProcessX11Events() _after_ the
>> GEaddDevice2(dd, devname). Option (b) is probably the easiest.
>> Paul: Does this analysis look roughly right?
>> -pd
>>> On 26 Apr 2019, at 01:23 , frede...@ofb.net wrote:
>>>
>>> Thanks Professor Dalgard.
>>>
>>> If you have a different way to fix the bug then I'd be happy to test
>>> it.
>>>
>>> Or whatever. I understand that maybe some data was being referenced
>>> before it had been initialized. I could also support moving the
>>> R_ProcessEvents call in another place, but it seems one would also
>>> like to generate some kind of warning message, at the location of the
>>> bad reference, rather than segfaulting. Was it not possible to
>>> identify this location? I'm guessing that Valgrind is a bit more
>>> mature now than it was in 2001...?
>>>
>>> Frederick
>>>
>>> On Wed, Apr 24, 2019 at 03:12:55PM +0200, peter dalgaard wrote:
>>>> OK, so I did the archaeology anyway....
>>>>
>>>>
>>>> This was the story, R-core November 29, 2001. Part of thread "X11 still
>>>> segfaults".
>>>>
>>>> ------------>>
>>>> .....
>>>> Gah. I've been too tired today. Why did that take me so long?
>>>>
>>>> The culprit seems to be
>>>>
>>>> R_ProcessEvents((void*) NULL)
>>>>
>>>> in newX11DeviceDriver
>>>>
>>>> This gets called *before* this stuff at the end of Rf_addX11Device
>>>>
>>>> dd = GEcreateDevDesc(dev);
>>>> addDevice((DevDesc*) dd);
>>>> initDisplayList((DevDesc*) dd);
>>>>
>>>> and it is that "dd" that gets called by Rf_playDisplayList. Removing
>>>> the offending line stops the segfaulting, seemingly with no ill
>>>> effects.
>>>>
>>>> I'm not really sure what the use of that line ever was; it might be
>>>> necessary to make the call somewhere later, but it appears to have
>>>> been possible to race past it before receiving any events all the
>>>> time.
>>>>
>>>> I also changed a couple of spots missing dd->newDevStruct=1
>>>>
>>>> Will commit in a moment.
>>>> <<------------
>>>>
>>>> And the following day, in "graphics saga part III", we had
>>>>
>>>> ------------->>
>>>> ...
>>>>
>>>> I can't make it happen in 1.3.1 but...
>>>>
>>>> It is probably not unrelated to the R_ProcessEvents line that
>>>> I took out, but that was definitely wrong. However, one might reenable
>>>> it if one could change this bit of code
>>>>
>>>> if (!(ptr_X11DeviceDriver)((DevDesc*)(dev), display, width, height, ps,
>>>> gamma,
>>>> colormodel, maxcubesize, canvascolor)) {
>>>> free(dev);
>>>> errorcall(gcall, "unable to start device %s", devname);
>>>> }
>>>> gsetVar(install(".Device"), mkString(devname), R_NilValue);
>>>> dd = GEcreateDevDesc(dev);
>>>> addDevice((DevDesc*) dd);
>>>> initDisplayList((DevDesc*) dd);
>>>>
>>>>
>>>> and put the if-clause last. A cursory clance through the three
>>>> functions that are being called didn't reveal anything that would rely
>>>> on having opened the device driver first.
>>>>
>>>> Paul?
>>>>
>>>> (I might try it locally, but I'm not sure I should commit anything.)
>>>>
>>>> <<-----------
>>>>
>>>> It seems that the suggestion was never followed up on?
>>>>
>>>> -pd
>>>>
>>>>
>>>>> On 24 Apr 2019, at 11:42 , peter dalgaard <pda...@gmail.com> wrote:
>>>>>
>>>>> I don't recall exactly what I did 18 years ago eiher and I likely don't
>>>>> have the time to dig into the archives and reconstruct.
>>>>>
>>>>> I can imagine that the issue had to do with the protocol around creating
>>>>> and mapping windows. Presumably the segfault comes from looking for
>>>>> events on a window that hasn't been created yet, or has already been
>>>>> destroyed, leading to a NULL reference somewhere. I have a vague
>>>>> recollection that the issue was window manager dependent (in 2001
>>>>> probably not twm, more likely xvwm on RedHat if it was affecting me).
>>>>>
>>>>> A proper fix should go via proper understanding of the X11 protocol -
>>>>> uncommenting a line is as bad as commenting it in the 1st place.... So
>>>>> more like "wait for window to exist THEN process events" -- but the 1st
>>>>> part may be WM specific, etc.
>>>>>
>>>>> I recall docs being quite obtuse, and the X11 "mechanism not policy"
>>>>> credo doesn't help as WMs are not obliged to (say) send notifications, so
>>>>> you can end up stalling, waiting for events that never happen.
>>>>>
>>>>> It is entirely possible that there is stuff in here that I didn't
>>>>> understand properly at the time, and still don't!
>>>>>
>>>>> - pd
>>>>>
>>>>>> On 24 Apr 2019, at 02:30 , Paul Murrell <p...@stat.auckland.ac.nz> wrote:
>>>>>>
>>>>>> Hi
>>>>>>
>>>>>> Sorry, I can't offer an explanation for the commented-out line.
>>>>>> However, regarding your final question of avoiding the R-core
>>>>>> bottleneck, you do have the option of creating a third-party graphics
>>>>>> device package. See, for example, the 'tikzDevice' and 'svglite'
>>>>>> packages on CRAN. Does that provide you with a way forward ?
>>>>>>
>>>>>> Paul
>>>>>>
>>>>>> On 20/04/2019 5:27 p.m., frede...@ofb.net wrote:
>>>>>>> Dear R Devel,
>>>>>>>
>>>>>>> I know that someone put this line in src/modules/X11/devX11.c:2824 for
>>>>>>> a reason, because commenting it out causes R to miss an important
>>>>>>> ConfigureNotify event in my window manager. The result is that plots
>>>>>>> are initially drawn off the window borders, unreadable.
>>>>>>>
>>>>>>> R_ProcessX11Events((void*) NULL);
>>>>>>>
>>>>>>> Unfortunately for me, this line is commented in the standard release
>>>>>>> of R, it has "#if BUG ... #endif" around it.
>>>>>>>
>>>>>>> I guess it is also unfortunate for anyone who uses the same window
>>>>>>> manager as I do, namely i3, which I think is pretty popular among Unix
>>>>>>> power users these days; not to mention other full-screen window
>>>>>>> managers which probably exhibit the same bug in R.
>>>>>>>
>>>>>>> Maybe everyone on the Core team uses twm as their window manager? Or
>>>>>>> RStudio on Windows? Which would be sad because then we're not
>>>>>>> representing an important user demographic, namely those who prefer
>>>>>>> software which is modern and powerful, yet simple to understand and
>>>>>>> modify; fully configurable and interoperable and so on.
>>>>>>>
>>>>>>> I first reported this bug 3 years ago. In doing research for my bug
>>>>>>> report, I found that the line was commented out by Peter Dalgaard in
>>>>>>> 2001 with the explanation "X11 segfault fix - I hope".
>>>>>>>
>>>>>>> I don't know what the way forward is. Obviously the Core Team has
>>>>>>> reason to say, "look, this isn't very important, it's been broken
>>>>>>> since 2001, maybe fixing it will cause the undocumented segfault bug
>>>>>>> to reappear, clearly no one here uses your window manager". Do I have
>>>>>>> to submit a correctness proof for the proposed change? What do I do?
>>>>>>>
>>>>>>> https://bugs.r-project.org/bugzilla/show_bug.cgi?id=16702
>>>>>>>
>>>>>>> As mentioned in my bug report, I checked using gdb that
>>>>>>> ConfigureNotify is indeed being received by the call to
>>>>>>> R_ProcessX11Events() when it is uncommented. I haven't experienced any
>>>>>>> segfaults.
>>>>>>>
>>>>>>> It's good that Peter left evidence that "R_ProcessX11Events" was being
>>>>>>> called 18 years ago from X11DeviceDriver(). If he had deleted the
>>>>>>> line, rather than commenting it, then discovering the reason for the
>>>>>>> window rendering bug would have been much harder for me.
>>>>>>>
>>>>>>> However, the downside is that now it is not just a matter of inserting
>>>>>>> the line where it belongs; I also feel a bit like I have to explain
>>>>>>> why it was initially removed. But although I've given it some thought,
>>>>>>> I still have no idea.
>>>>>>>
>>>>>>> Somewhat tangentially, I am wondering if there is some way that we
>>>>>>> could make the development of R's graphics code proceed at a faster
>>>>>>> rate, for example by pulling it out into a separate module, so that
>>>>>>> people could offer alternative implementations via CRAN etc., rather
>>>>>>> than having R Core be the bottleneck. Would this make sense? Has it
>>>>>>> already been done?
>>>>>>>
>>>>>>> Thank you,
>>>>>>>
>>>>>>> Frederick
>>>>>>>
>>>>>>> ______________________________________________
>>>>>>> R-devel@r-project.org mailing list
>>>>>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>>>>>
>>>>>> --
>>>>>> Dr Paul Murrell
>>>>>> Department of Statistics
>>>>>> The University of Auckland
>>>>>> Private Bag 92019
>>>>>> Auckland
>>>>>> New Zealand
>>>>>> 64 9 3737599 x85392
>>>>>> p...@stat.auckland.ac.nz
>>>>>> http://www.stat.auckland.ac.nz/~paul/
>>>>>>
>>>>>> ______________________________________________
>>>>>> R-devel@r-project.org mailing list
>>>>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>>>>
>>>>> --
>>>>> Peter Dalgaard, Professor,
>>>>> Center for Statistics, Copenhagen Business School
>>>>> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
>>>>> Phone: (+45)38153501
>>>>> Office: A 4.23
>>>>> Email: pd....@cbs.dk Priv: pda...@gmail.com
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>> --
>>>> Peter Dalgaard, Professor,
>>>> Center for Statistics, Copenhagen Business School
>>>> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
>>>> Phone: (+45)38153501
>>>> Office: A 4.23
>>>> Email: pd....@cbs.dk Priv: pda...@gmail.com
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>
> --
> Dr Paul Murrell
> Department of Statistics
> The University of Auckland
> Private Bag 92019
> Auckland
> New Zealand
> 64 9 3737599 x85392
> p...@stat.auckland.ac.nz
> http://www.stat.auckland.ac.nz/~paul/
--
Peter Dalgaard, Professor,
Center for Statistics, Copenhagen Business School
Solbjerg Plads 3, 2000 Frederiksberg, Denmark
Phone: (+45)38153501
Office: A 4.23
Email: pd....@cbs.dk Priv: pda...@gmail.com
______________________________________________
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
