use of the tcltk package crashes R 4.0.1 for Windows

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

use of the tcltk package crashes R 4.0.1 for Windows

Fox, John
Hi,

The following code, from the examples in ?TkWidgets , immediately crashes R 4.0.1 for Windows:

--------------------- snip --------------------
library("tcltk")
tt <- tktoplevel()
label.widget <- tklabel(tt, text = "Hello, World!")
button.widget <- tkbutton(tt, text = "Push",
         command = function()cat("OW!\n"))
tkpack(label.widget, button.widget) # geometry manager
--------------------- snip --------------------

Session info (prior to the crash):

--------------------- snip --------------------
> sessionInfo()
R version 4.0.1 (2020-06-06)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 18363)

Matrix products: default

locale:
[1] LC_COLLATE=English_United States.1252
[2] LC_CTYPE=English_United States.1252  
[3] LC_MONETARY=English_United States.1252
[4] LC_NUMERIC=C                          
[5] LC_TIME=English_United States.1252    

attached base packages:
[1] tcltk     stats     graphics  grDevices utils     datasets  methods  
[8] base    

loaded via a namespace (and not attached):
[1] compiler_4.0.1 tools_4.0.1  
--------------------- snip --------------------

I observe this behaviour both in the Rgui and when I run R in a terminal. I think the problem is general to the use of the tcltk package.

Best,
 John

-----------------------------------------------------------------
John Fox
Professor Emeritus
McMaster University
Hamilton, Ontario, Canada
Web: https://socialsciences.mcmaster.ca/jfox/

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: use of the tcltk package crashes R 4.0.1 for Windows

Peter Dalgaard-2
Not happening on Mac, so likely a Windows build issue.

(There's no 4.0.1 CRAN package yet, and no nightly build of 4.0.1 Patched, but the only thing changed in the sources since r78644 is the VERSION file.)

-pd

> On 7 Jun 2020, at 03:13 , Fox, John <[hidden email]> wrote:
>
> Hi,
>
> The following code, from the examples in ?TkWidgets , immediately crashes R 4.0.1 for Windows:
>
> --------------------- snip --------------------
> library("tcltk")
> tt <- tktoplevel()
> label.widget <- tklabel(tt, text = "Hello, World!")
> button.widget <- tkbutton(tt, text = "Push",
> command = function()cat("OW!\n"))
> tkpack(label.widget, button.widget) # geometry manager
> --------------------- snip --------------------
>
> Session info (prior to the crash):
>
> --------------------- snip --------------------
>> sessionInfo()
> R version 4.0.1 (2020-06-06)
> Platform: x86_64-w64-mingw32/x64 (64-bit)
> Running under: Windows 10 x64 (build 18363)
>
> Matrix products: default
>
> locale:
> [1] LC_COLLATE=English_United States.1252
> [2] LC_CTYPE=English_United States.1252  
> [3] LC_MONETARY=English_United States.1252
> [4] LC_NUMERIC=C                          
> [5] LC_TIME=English_United States.1252    
>
> attached base packages:
> [1] tcltk     stats     graphics  grDevices utils     datasets  methods  
> [8] base    
>
> loaded via a namespace (and not attached):
> [1] compiler_4.0.1 tools_4.0.1  
> --------------------- snip --------------------
>
> I observe this behaviour both in the Rgui and when I run R in a terminal. I think the problem is general to the use of the tcltk package.
>
> Best,
> John
>
> -----------------------------------------------------------------
> John Fox
> Professor Emeritus
> McMaster University
> Hamilton, Ontario, Canada
> Web: https://socialsciences.mcmaster.ca/jfox/
>
> ______________________________________________
> [hidden email] 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: [hidden email]  Priv: [hidden email]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: use of the tcltk package crashes R 4.0.1 for Windows

Peter Dalgaard-2
Just to be clear, I was talking about Mac binary packages. The one available and tested was

R-4.0-branch
4.0.1 RC
(2020/05/30, r78644)

from mac.r-project.org. (Simon Urbanek out of office for the weekend, I suppose.)

-pd

> On 7 Jun 2020, at 08:27 , peter dalgaard <[hidden email]> wrote:
>
> Not happening on Mac, so likely a Windows build issue.
>
> (There's no 4.0.1 CRAN package yet, and no nightly build of 4.0.1 Patched, but the only thing changed in the sources since r78644 is the VERSION file.)
>
> -pd
>
>> On 7 Jun 2020, at 03:13 , Fox, John <[hidden email]> wrote:
>>
>> Hi,
>>
>> The following code, from the examples in ?TkWidgets , immediately crashes R 4.0.1 for Windows:
>>
>> --------------------- snip --------------------
>> library("tcltk")
>> tt <- tktoplevel()
>> label.widget <- tklabel(tt, text = "Hello, World!")
>> button.widget <- tkbutton(tt, text = "Push",
>> command = function()cat("OW!\n"))
>> tkpack(label.widget, button.widget) # geometry manager
>> --------------------- snip --------------------
>>
>> Session info (prior to the crash):
>>
>> --------------------- snip --------------------
>>> sessionInfo()
>> R version 4.0.1 (2020-06-06)
>> Platform: x86_64-w64-mingw32/x64 (64-bit)
>> Running under: Windows 10 x64 (build 18363)
>>
>> Matrix products: default
>>
>> locale:
>> [1] LC_COLLATE=English_United States.1252
>> [2] LC_CTYPE=English_United States.1252  
>> [3] LC_MONETARY=English_United States.1252
>> [4] LC_NUMERIC=C                          
>> [5] LC_TIME=English_United States.1252    
>>
>> attached base packages:
>> [1] tcltk     stats     graphics  grDevices utils     datasets  methods  
>> [8] base    
>>
>> loaded via a namespace (and not attached):
>> [1] compiler_4.0.1 tools_4.0.1  
>> --------------------- snip --------------------
>>
>> I observe this behaviour both in the Rgui and when I run R in a terminal. I think the problem is general to the use of the tcltk package.
>>
>> Best,
>> John
>>
>> -----------------------------------------------------------------
>> John Fox
>> Professor Emeritus
>> McMaster University
>> Hamilton, Ontario, Canada
>> Web: https://socialsciences.mcmaster.ca/jfox/
>>
>> ______________________________________________
>> [hidden email] 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: [hidden email]  Priv: [hidden email]
>
>
>
>
>
>
>
>
>

--
Peter Dalgaard, Professor,
Center for Statistics, Copenhagen Business School
Solbjerg Plads 3, 2000 Frederiksberg, Denmark
Phone: (+45)38153501
Office: A 4.23
Email: [hidden email]  Priv: [hidden email]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: use of the tcltk package crashes R 4.0.1 for Windows

Peter Dalgaard-2
In reply to this post by Peter Dalgaard-2
John,

The Windows installation instructions document has the following. So, one obvious question is whether you did select it. (I haven't installed on WIndows for ages, so I don't know whether this was changed recently or even whether the selection is on or off by default).

-pd

Using package tcltk
===================

The package tcltk supports building graphical interfaces with Tcl/Tk.
"Support Files for Package tcltk" needs to be selected from the
installer for this to work; alternatively you can use an existing
installation of Tcl/Tk 8.6.x by following the instructions in the
rw-FAQ.




> On 7 Jun 2020, at 08:27 , peter dalgaard <[hidden email]> wrote:
>
> Not happening on Mac, so likely a Windows build issue.
>
> (There's no 4.0.1 CRAN package yet, and no nightly build of 4.0.1 Patched, but the only thing changed in the sources since r78644 is the VERSION file.)
>
> -pd
>
>> On 7 Jun 2020, at 03:13 , Fox, John <[hidden email]> wrote:
>>
>> Hi,
>>
>> The following code, from the examples in ?TkWidgets , immediately crashes R 4.0.1 for Windows:
>>
>> --------------------- snip --------------------
>> library("tcltk")
>> tt <- tktoplevel()
>> label.widget <- tklabel(tt, text = "Hello, World!")
>> button.widget <- tkbutton(tt, text = "Push",
>> command = function()cat("OW!\n"))
>> tkpack(label.widget, button.widget) # geometry manager
>> --------------------- snip --------------------
>>
>> Session info (prior to the crash):
>>
>> --------------------- snip --------------------
>>> sessionInfo()
>> R version 4.0.1 (2020-06-06)
>> Platform: x86_64-w64-mingw32/x64 (64-bit)
>> Running under: Windows 10 x64 (build 18363)
>>
>> Matrix products: default
>>
>> locale:
>> [1] LC_COLLATE=English_United States.1252
>> [2] LC_CTYPE=English_United States.1252  
>> [3] LC_MONETARY=English_United States.1252
>> [4] LC_NUMERIC=C                          
>> [5] LC_TIME=English_United States.1252    
>>
>> attached base packages:
>> [1] tcltk     stats     graphics  grDevices utils     datasets  methods  
>> [8] base    
>>
>> loaded via a namespace (and not attached):
>> [1] compiler_4.0.1 tools_4.0.1  
>> --------------------- snip --------------------
>>
>> I observe this behaviour both in the Rgui and when I run R in a terminal. I think the problem is general to the use of the tcltk package.
>>
>> Best,
>> John
>>
>> -----------------------------------------------------------------
>> John Fox
>> Professor Emeritus
>> McMaster University
>> Hamilton, Ontario, Canada
>> Web: https://socialsciences.mcmaster.ca/jfox/
>>
>> ______________________________________________
>> [hidden email] 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: [hidden email]  Priv: [hidden email]
>
>
>
>
>
>
>
>
>

--
Peter Dalgaard, Professor,
Center for Statistics, Copenhagen Business School
Solbjerg Plads 3, 2000 Frederiksberg, Denmark
Phone: (+45)38153501
Office: A 4.23
Email: [hidden email]  Priv: [hidden email]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: use of the tcltk package crashes R 4.0.1 for Windows

Fox, John
Dear Peter,

First, thank you for following up on this problem.

Unless I somehow inexplicably missed it, as I just confirmed, the R 4.0.1 Windows installer *doesn't* ask to install support files for Tcl/Tk.

Nor am I only one to notice this problem. I was made aware of it when several Rcmdr users wrote to me yesterday to say that the package was crashing R when it loads.

Finally, even if Tcl/Tk support is now a non-default option in R for Windows, R shouldn't crash if Tcl/Tk isn't installed.

Best,
 John

> On Jun 7, 2020, at 2:44 AM, peter dalgaard <[hidden email]> wrote:
>
> John,
>
> The Windows installation instructions document has the following. So, one obvious question is whether you did select it. (I haven't installed on WIndows for ages, so I don't know whether this was changed recently or even whether the selection is on or off by default).
>
> -pd
>
> Using package tcltk
> ===================
>
> The package tcltk supports building graphical interfaces with Tcl/Tk.
> "Support Files for Package tcltk" needs to be selected from the
> installer for this to work; alternatively you can use an existing
> installation of Tcl/Tk 8.6.x by following the instructions in the
> rw-FAQ.
>
>
>
>
>> On 7 Jun 2020, at 08:27 , peter dalgaard <[hidden email]> wrote:
>>
>> Not happening on Mac, so likely a Windows build issue.
>>
>> (There's no 4.0.1 CRAN package yet, and no nightly build of 4.0.1 Patched, but the only thing changed in the sources since r78644 is the VERSION file.)
>>
>> -pd
>>
>>> On 7 Jun 2020, at 03:13 , Fox, John <[hidden email]> wrote:
>>>
>>> Hi,
>>>
>>> The following code, from the examples in ?TkWidgets , immediately crashes R 4.0.1 for Windows:
>>>
>>> --------------------- snip --------------------
>>> library("tcltk")
>>> tt <- tktoplevel()
>>> label.widget <- tklabel(tt, text = "Hello, World!")
>>> button.widget <- tkbutton(tt, text = "Push",
>>> command = function()cat("OW!\n"))
>>> tkpack(label.widget, button.widget) # geometry manager
>>> --------------------- snip --------------------
>>>
>>> Session info (prior to the crash):
>>>
>>> --------------------- snip --------------------
>>>> sessionInfo()
>>> R version 4.0.1 (2020-06-06)
>>> Platform: x86_64-w64-mingw32/x64 (64-bit)
>>> Running under: Windows 10 x64 (build 18363)
>>>
>>> Matrix products: default
>>>
>>> locale:
>>> [1] LC_COLLATE=English_United States.1252
>>> [2] LC_CTYPE=English_United States.1252  
>>> [3] LC_MONETARY=English_United States.1252
>>> [4] LC_NUMERIC=C                          
>>> [5] LC_TIME=English_United States.1252    
>>>
>>> attached base packages:
>>> [1] tcltk     stats     graphics  grDevices utils     datasets  methods  
>>> [8] base    
>>>
>>> loaded via a namespace (and not attached):
>>> [1] compiler_4.0.1 tools_4.0.1  
>>> --------------------- snip --------------------
>>>
>>> I observe this behaviour both in the Rgui and when I run R in a terminal. I think the problem is general to the use of the tcltk package.
>>>
>>> Best,
>>> John
>>>
>>> -----------------------------------------------------------------
>>> John Fox
>>> Professor Emeritus
>>> McMaster University
>>> Hamilton, Ontario, Canada
>>> Web: https://socialsciences.mcmaster.ca/jfox/
>>>
>>> ______________________________________________
>>> [hidden email] 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: [hidden email]  Priv: [hidden email]
>>
>>
>>
>>
>>
>>
>>
>>
>>
>
> --
> Peter Dalgaard, Professor,
> Center for Statistics, Copenhagen Business School
> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
> Phone: (+45)38153501
> Office: A 4.23
> Email: [hidden email]  Priv: [hidden email]
>
>
>
>
>
>
>
>
>

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: use of the tcltk package crashes R 4.0.1 for Windows

Jeroen Ooms
In reply to this post by Fox, John
On Sun, Jun 7, 2020 at 3:13 AM Fox, John <[hidden email]> wrote:

>
> Hi,
>
> The following code, from the examples in ?TkWidgets , immediately crashes R 4.0.1 for Windows:
>
> --------------------- snip --------------------
> library("tcltk")
> tt <- tktoplevel()
> label.widget <- tklabel(tt, text = "Hello, World!")
> button.widget <- tkbutton(tt, text = "Push",
>          command = function()cat("OW!\n"))
> tkpack(label.widget, button.widget) # geometry manager
> --------------------- snip --------------------


I can reproduce this. The backtrace shows the crash happens in
dotTclObjv  [/src/library/tcltk/src/tcltk.c@243 ]. This looks like a
bug that was introduced by commit 78408/78409 about a month ago. I
think the problem is that this commit changes 'calloc' to 'Calloc'
without changing the corresponding 'free' to 'Free'.

This has nothing to do with the Windows build or installation. Nothing
has changed in the windows build procedure between 4.0.0 and 4.0.1.

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: use of the tcltk package crashes R 4.0.1 for Windows

Fox, John
Dear Jeroen,

Thank you for tracking down the source of the problem.

You probably saw that Peter Dalgaard reported that the tcltk package apparently is working fine in R 4.0.1 on macOS. I haven't confirmed that myself because the Mac binary for R 4.0.1 isn't yet on CRAN.

Best,
 John

> On Jun 7, 2020, at 10:00 AM, Jeroen Ooms <[hidden email]> wrote:
>
> On Sun, Jun 7, 2020 at 3:13 AM Fox, John <[hidden email]> wrote:
>>
>> Hi,
>>
>> The following code, from the examples in ?TkWidgets , immediately crashes R 4.0.1 for Windows:
>>
>> --------------------- snip --------------------
>> library("tcltk")
>> tt <- tktoplevel()
>> label.widget <- tklabel(tt, text = "Hello, World!")
>> button.widget <- tkbutton(tt, text = "Push",
>>         command = function()cat("OW!\n"))
>> tkpack(label.widget, button.widget) # geometry manager
>> --------------------- snip --------------------
>
>
> I can reproduce this. The backtrace shows the crash happens in
> dotTclObjv  [/src/library/tcltk/src/tcltk.c@243 ]. This looks like a
> bug that was introduced by commit 78408/78409 about a month ago. I
> think the problem is that this commit changes 'calloc' to 'Calloc'
> without changing the corresponding 'free' to 'Free'.
>
> This has nothing to do with the Windows build or installation. Nothing
> has changed in the windows build procedure between 4.0.0 and 4.0.1.

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: use of the tcltk package crashes R 4.0.1 for Windows

Duncan Murdoch-2
In reply to this post by Jeroen Ooms
On 07/06/2020 10:00 a.m., Jeroen Ooms wrote:

> On Sun, Jun 7, 2020 at 3:13 AM Fox, John <[hidden email]> wrote:
>>
>> Hi,
>>
>> The following code, from the examples in ?TkWidgets , immediately crashes R 4.0.1 for Windows:
>>
>> --------------------- snip --------------------
>> library("tcltk")
>> tt <- tktoplevel()
>> label.widget <- tklabel(tt, text = "Hello, World!")
>> button.widget <- tkbutton(tt, text = "Push",
>>           command = function()cat("OW!\n"))
>> tkpack(label.widget, button.widget) # geometry manager
>> --------------------- snip --------------------
>
>
> I can reproduce this. The backtrace shows the crash happens in
> dotTclObjv  [/src/library/tcltk/src/tcltk.c@243 ]. This looks like a
> bug that was introduced by commit 78408/78409 about a month ago. I
> think the problem is that this commit changes 'calloc' to 'Calloc'
> without changing the corresponding 'free' to 'Free'.

The same commit made the same kind of change to unix/sys-std.c as well.

Duncan Murdoch

>
> This has nothing to do with the Windows build or installation. Nothing
> has changed in the windows build procedure between 4.0.0 and 4.0.1.
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: use of the tcltk package crashes R 4.0.1 for Windows

Peter Dalgaard-2
In reply to this post by Jeroen Ooms
So this wasn't tested for a month?

Anyways, Free() is just free() with a check that we're not freeing a null pointer, followed by setting the pointer to NULL. At that point of tcltk.c, we have

   for (objc = i = 0; i < length(avec); i++){
        const char *s;
        char *tmp;
        if (!isNull(nm) && strlen(s = translateChar(STRING_ELT(nm, i)))){
            //  tmp = calloc(strlen(s)+2, sizeof(char));
            tmp = Calloc(strlen(s)+2, char);
            *tmp = '-';
            strcpy(tmp+1, s);
            objv[objc++] = Tcl_NewStringObj(tmp, -1);
            free(tmp);
        }
        if (!isNull(t = VECTOR_ELT(avec, i)))
            objv[objc++] = (Tcl_Obj *) R_ExternalPtrAddr(t);
    }

and I can't see how tmp can be NULL at the free(), nor can I see it mattering if it is not set to NULL (notice that it goes out of scope with the for loop).

-pd
 

> On 7 Jun 2020, at 16:00 , Jeroen Ooms <[hidden email]> wrote:
>
> On Sun, Jun 7, 2020 at 3:13 AM Fox, John <[hidden email]> wrote:
>>
>> Hi,
>>
>> The following code, from the examples in ?TkWidgets , immediately crashes R 4.0.1 for Windows:
>>
>> --------------------- snip --------------------
>> library("tcltk")
>> tt <- tktoplevel()
>> label.widget <- tklabel(tt, text = "Hello, World!")
>> button.widget <- tkbutton(tt, text = "Push",
>>         command = function()cat("OW!\n"))
>> tkpack(label.widget, button.widget) # geometry manager
>> --------------------- snip --------------------
>
>
> I can reproduce this. The backtrace shows the crash happens in
> dotTclObjv  [/src/library/tcltk/src/tcltk.c@243 ]. This looks like a
> bug that was introduced by commit 78408/78409 about a month ago. I
> think the problem is that this commit changes 'calloc' to 'Calloc'
> without changing the corresponding 'free' to 'Free'.
>
> This has nothing to do with the Windows build or installation. Nothing
> has changed in the windows build procedure between 4.0.0 and 4.0.1.
>
> ______________________________________________
> [hidden email] 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: [hidden email]  Priv: [hidden email]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [External] Re: use of the tcltk package crashes R 4.0.1 for Windows

luke-tierney
On Sun, 7 Jun 2020, peter dalgaard wrote:

> So this wasn't tested for a month?
>
> Anyways, Free() is just free() with a check that we're not freeing a null pointer, followed by setting the pointer to NULL. At that point of tcltk.c, we have
>
>   for (objc = i = 0; i < length(avec); i++){
>        const char *s;
>        char *tmp;
>        if (!isNull(nm) && strlen(s = translateChar(STRING_ELT(nm, i)))){
>            //  tmp = calloc(strlen(s)+2, sizeof(char));
>            tmp = Calloc(strlen(s)+2, char);
>            *tmp = '-';
>            strcpy(tmp+1, s);
>            objv[objc++] = Tcl_NewStringObj(tmp, -1);
>            free(tmp);
>        }
>        if (!isNull(t = VECTOR_ELT(avec, i)))
>            objv[objc++] = (Tcl_Obj *) R_ExternalPtrAddr(t);
>    }
>
> and I can't see how tmp can be NULL at the free(), nor can I see it mattering if it is not set to NULL (notice that it goes out of scope with the for loop).

Right. And the calloc->Calloc change doesn't look like an issue either
-- just checking for a NULL.

If the crash is happening in free() then that most likely means
corrupted malloc data structures. Unfortunately that could be
happening anywhere.

Best bet to narrow this down is for someone with a good Windows setup
who can reproduce this to bisect the svn commits and see at what
commit this started happening. Unfortunately my office Windows machine
isn't responding and it will probably take some time to get that
fixed.

Best,

luke

>
> -pd
>
>
>> On 7 Jun 2020, at 16:00 , Jeroen Ooms <[hidden email]> wrote:
>>
>> On Sun, Jun 7, 2020 at 3:13 AM Fox, John <[hidden email]> wrote:
>>>
>>> Hi,
>>>
>>> The following code, from the examples in ?TkWidgets , immediately crashes R 4.0.1 for Windows:
>>>
>>> --------------------- snip --------------------
>>> library("tcltk")
>>> tt <- tktoplevel()
>>> label.widget <- tklabel(tt, text = "Hello, World!")
>>> button.widget <- tkbutton(tt, text = "Push",
>>>         command = function()cat("OW!\n"))
>>> tkpack(label.widget, button.widget) # geometry manager
>>> --------------------- snip --------------------
>>
>>
>> I can reproduce this. The backtrace shows the crash happens in
>> dotTclObjv  [/src/library/tcltk/src/tcltk.c@243 ]. This looks like a
>> bug that was introduced by commit 78408/78409 about a month ago. I
>> think the problem is that this commit changes 'calloc' to 'Calloc'
>> without changing the corresponding 'free' to 'Free'.
>>
>> This has nothing to do with the Windows build or installation. Nothing
>> has changed in the windows build procedure between 4.0.0 and 4.0.1.
>>
>> ______________________________________________
>> [hidden email] mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>
>

--
Luke Tierney
Ralph E. Wareham Professor of Mathematical Sciences
University of Iowa                  Phone:             319-335-3386
Department of Statistics and        Fax:               319-335-3017
    Actuarial Science
241 Schaeffer Hall                  email:   [hidden email]
Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [External] Re: use of the tcltk package crashes R 4.0.1 for Windows

Fox, John
Hi,

Does it make sense to withdraw the Windows R 4.0.1 binary until the issue is resolved?

Best,
 John

> -----Original Message-----
> From: [hidden email] <[hidden email]>
> Sent: Sunday, June 7, 2020 11:54 AM
> To: peter dalgaard <[hidden email]>
> Cc: Jeroen Ooms <[hidden email]>; Fox, John <[hidden email]>; r-
> [hidden email]
> Subject: Re: [External] Re: [Rd] use of the tcltk package crashes R 4.0.1
> for Windows
>
> On Sun, 7 Jun 2020, peter dalgaard wrote:
>
> > So this wasn't tested for a month?
> >
> > Anyways, Free() is just free() with a check that we're not freeing a
> > null pointer, followed by setting the pointer to NULL. At that point
> > of tcltk.c, we have
> >
> >   for (objc = i = 0; i < length(avec); i++){
> >        const char *s;
> >        char *tmp;
> >        if (!isNull(nm) && strlen(s = translateChar(STRING_ELT(nm, i)))){
> >            //  tmp = calloc(strlen(s)+2, sizeof(char));
> >            tmp = Calloc(strlen(s)+2, char);
> >            *tmp = '-';
> >            strcpy(tmp+1, s);
> >            objv[objc++] = Tcl_NewStringObj(tmp, -1);
> >            free(tmp);
> >        }
> >        if (!isNull(t = VECTOR_ELT(avec, i)))
> >            objv[objc++] = (Tcl_Obj *) R_ExternalPtrAddr(t);
> >    }
> >
> > and I can't see how tmp can be NULL at the free(), nor can I see it
> mattering if it is not set to NULL (notice that it goes out of scope with
> the for loop).
>
> Right. And the calloc->Calloc change doesn't look like an issue either
> -- just checking for a NULL.
>
> If the crash is happening in free() then that most likely means corrupted
> malloc data structures. Unfortunately that could be happening anywhere.
>
> Best bet to narrow this down is for someone with a good Windows setup who
> can reproduce this to bisect the svn commits and see at what commit this
> started happening. Unfortunately my office Windows machine isn't
> responding and it will probably take some time to get that fixed.
>
> Best,
>
> luke
>
> >
> > -pd
> >
> >
> >> On 7 Jun 2020, at 16:00 , Jeroen Ooms <[hidden email]> wrote:
> >>
> >> On Sun, Jun 7, 2020 at 3:13 AM Fox, John <[hidden email]> wrote:
> >>>
> >>> Hi,
> >>>
> >>> The following code, from the examples in ?TkWidgets , immediately
> crashes R 4.0.1 for Windows:
> >>>
> >>> --------------------- snip --------------------
> >>> library("tcltk")
> >>> tt <- tktoplevel()
> >>> label.widget <- tklabel(tt, text = "Hello, World!") button.widget <-
> >>> tkbutton(tt, text = "Push",
> >>>         command = function()cat("OW!\n")) tkpack(label.widget,
> >>> button.widget) # geometry manager
> >>> --------------------- snip --------------------
> >>
> >>
> >> I can reproduce this. The backtrace shows the crash happens in
> >> dotTclObjv  [/src/library/tcltk/src/tcltk.c@243 ]. This looks like a
> >> bug that was introduced by commit 78408/78409 about a month ago. I
> >> think the problem is that this commit changes 'calloc' to 'Calloc'
> >> without changing the corresponding 'free' to 'Free'.
> >>
> >> This has nothing to do with the Windows build or installation.
> >> Nothing has changed in the windows build procedure between 4.0.0 and
> 4.0.1.
> >>
> >> ______________________________________________
> >> [hidden email] mailing list
> >> https://stat.ethz.ch/mailman/listinfo/r-devel
> >
> >
>
> --
> Luke Tierney
> Ralph E. Wareham Professor of Mathematical Sciences
> University of Iowa                  Phone:             319-335-3386
> Department of Statistics and        Fax:               319-335-3017
>     Actuarial Science
> 241 Schaeffer Hall                  email:   [hidden email]
> Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [External] Re: use of the tcltk package crashes R 4.0.1 for Windows

luke-tierney
In reply to this post by luke-tierney
There is one other possibility:

It may be that the calloc/free pair picked up by the tcltk package DLL
is different from the pair picked up when building base R. (We provide
our own malloc framework, but if the macros aren't quite right it may
be that the system malloc is picked up in some cases). In that case
using Calloc and free would be mismatching the malloc systems and
probably segfault.

If that is indeed happening we should fix it, but using Free with
Calloc should cure the immediate symptom.

Best,

luke

On Sun, 7 Jun 2020, [hidden email] wrote:

> On Sun, 7 Jun 2020, peter dalgaard wrote:
>
>> So this wasn't tested for a month?
>>
>> Anyways, Free() is just free() with a check that we're not freeing a null
>> pointer, followed by setting the pointer to NULL. At that point of tcltk.c,
>> we have
>>
>>   for (objc = i = 0; i < length(avec); i++){
>>        const char *s;
>>        char *tmp;
>>        if (!isNull(nm) && strlen(s = translateChar(STRING_ELT(nm, i)))){
>>            //  tmp = calloc(strlen(s)+2, sizeof(char));
>>            tmp = Calloc(strlen(s)+2, char);
>>            *tmp = '-';
>>            strcpy(tmp+1, s);
>>            objv[objc++] = Tcl_NewStringObj(tmp, -1);
>>            free(tmp);
>>        }
>>        if (!isNull(t = VECTOR_ELT(avec, i)))
>>            objv[objc++] = (Tcl_Obj *) R_ExternalPtrAddr(t);
>>    }
>>
>> and I can't see how tmp can be NULL at the free(), nor can I see it
>> mattering if it is not set to NULL (notice that it goes out of scope with
>> the for loop).
>
> Right. And the calloc->Calloc change doesn't look like an issue either
> -- just checking for a NULL.
>
> If the crash is happening in free() then that most likely means
> corrupted malloc data structures. Unfortunately that could be
> happening anywhere.
>
> Best bet to narrow this down is for someone with a good Windows setup
> who can reproduce this to bisect the svn commits and see at what
> commit this started happening. Unfortunately my office Windows machine
> isn't responding and it will probably take some time to get that
> fixed.
>
> Best,
>
> luke
>
>>
>> -pd
>>
>>
>>> On 7 Jun 2020, at 16:00 , Jeroen Ooms <[hidden email]> wrote:
>>>
>>> On Sun, Jun 7, 2020 at 3:13 AM Fox, John <[hidden email]> wrote:
>>>>
>>>> Hi,
>>>>
>>>> The following code, from the examples in ?TkWidgets , immediately crashes
>>>> R 4.0.1 for Windows:
>>>>
>>>> --------------------- snip --------------------
>>>> library("tcltk")
>>>> tt <- tktoplevel()
>>>> label.widget <- tklabel(tt, text = "Hello, World!")
>>>> button.widget <- tkbutton(tt, text = "Push",
>>>>         command = function()cat("OW!\n"))
>>>> tkpack(label.widget, button.widget) # geometry manager
>>>> --------------------- snip --------------------
>>>
>>>
>>> I can reproduce this. The backtrace shows the crash happens in
>>> dotTclObjv  [/src/library/tcltk/src/tcltk.c@243 ]. This looks like a
>>> bug that was introduced by commit 78408/78409 about a month ago. I
>>> think the problem is that this commit changes 'calloc' to 'Calloc'
>>> without changing the corresponding 'free' to 'Free'.
>>>
>>> This has nothing to do with the Windows build or installation. Nothing
>>> has changed in the windows build procedure between 4.0.0 and 4.0.1.
>>>
>>> ______________________________________________
>>> [hidden email] mailing list
>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>
>>
>
>

--
Luke Tierney
Ralph E. Wareham Professor of Mathematical Sciences
University of Iowa                  Phone:             319-335-3386
Department of Statistics and        Fax:               319-335-3017
    Actuarial Science
241 Schaeffer Hall                  email:   [hidden email]
Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [External] Re: use of the tcltk package crashes R 4.0.1 for Windows

Jeroen Ooms
In reply to this post by luke-tierney
On Sun, Jun 7, 2020 at 5:53 PM <[hidden email]> wrote:

>
> On Sun, 7 Jun 2020, peter dalgaard wrote:
>
> > So this wasn't tested for a month?
> >
> > Anyways, Free() is just free() with a check that we're not freeing a null pointer, followed by setting the pointer to NULL. At that point of tcltk.c, we have
> >
> >   for (objc = i = 0; i < length(avec); i++){
> >        const char *s;
> >        char *tmp;
> >        if (!isNull(nm) && strlen(s = translateChar(STRING_ELT(nm, i)))){
> >            //  tmp = calloc(strlen(s)+2, sizeof(char));
> >            tmp = Calloc(strlen(s)+2, char);
> >            *tmp = '-';
> >            strcpy(tmp+1, s);
> >            objv[objc++] = Tcl_NewStringObj(tmp, -1);
> >            free(tmp);
> >        }
> >        if (!isNull(t = VECTOR_ELT(avec, i)))
> >            objv[objc++] = (Tcl_Obj *) R_ExternalPtrAddr(t);
> >    }
> >
> > and I can't see how tmp can be NULL at the free(), nor can I see it mattering if it is not set to NULL (notice that it goes out of scope with the for loop).
>
> Right. And the calloc->Calloc change doesn't look like an issue either
> -- just checking for a NULL.
>
> If the crash is happening in free() then that most likely means
> corrupted malloc data structures. Unfortunately that could be
> happening anywhere.

Writing R extensions, section 6.1.2 says: "Do not assume that memory
allocated by Calloc/Realloc comes from the same pool as used by
malloc: in particular do not use free or strdup with it."

I think the reason is that R uses dlmalloc for Calloc on Windows:
https://github.com/wch/r-source/blob/c634fec5214e73747b44d7c0e6f047fefe44667d/src/main/memory.c#L94-L103

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [External] use of the tcltk package crashes R 4.0.1 for Windows

Peter Dalgaard-2
In reply to this post by luke-tierney


> On 7 Jun 2020, at 17:53 , [hidden email] wrote:
>
> On Sun, 7 Jun 2020, peter dalgaard wrote:
>
>> So this wasn't tested for a month?
>>
>> Anyways, Free() is just free() with a check that we're not freeing a null pointer, followed by setting the pointer to NULL. At that point of tcltk.c, we have
>>
>>  for (objc = i = 0; i < length(avec); i++){
>>       const char *s;
>>       char *tmp;
>>       if (!isNull(nm) && strlen(s = translateChar(STRING_ELT(nm, i)))){
>>           //  tmp = calloc(strlen(s)+2, sizeof(char));
>>           tmp = Calloc(strlen(s)+2, char);
>>           *tmp = '-';
>>           strcpy(tmp+1, s);
>>           objv[objc++] = Tcl_NewStringObj(tmp, -1);
>>           free(tmp);
>>       }
>>       if (!isNull(t = VECTOR_ELT(avec, i)))
>>           objv[objc++] = (Tcl_Obj *) R_ExternalPtrAddr(t);
>>   }
>>
>> and I can't see how tmp can be NULL at the free(), nor can I see it mattering if it is not set to NULL (notice that it goes out of scope with the for loop).
>
> Right. And the calloc->Calloc change doesn't look like an issue either
> -- just checking for a NULL.
>
> If the crash is happening in free() then that most likely means
> corrupted malloc data structures. Unfortunately that could be
> happening anywhere.
>
> Best bet to narrow this down is for someone with a good Windows setup
> who can reproduce this to bisect the svn commits and see at what
> commit this started happening. Unfortunately my office Windows machine
> isn't responding and it will probably take some time to get that
> fixed.

Also, it is possible that the issue is really a line or two earlier, so it would be good to get in with a debugger and see what is actually in *tmp and objv[objc++] at the point of the crash.

Also, Tcl_NewStringObj(tmp, -1) obviously must allocate, but it would be rather odd if it didn't use the system allocator (Tcl is designed to be embeddable, the only strange thing R does in that respect is the marriage of the two event loops).

-pd


>
> Best,
>
> luke
>
>>
>> -pd
>>
>>
>>> On 7 Jun 2020, at 16:00 , Jeroen Ooms <[hidden email]> wrote:
>>>
>>> On Sun, Jun 7, 2020 at 3:13 AM Fox, John <[hidden email]> wrote:
>>>>
>>>> Hi,
>>>>
>>>> The following code, from the examples in ?TkWidgets , immediately crashes R 4.0.1 for Windows:
>>>>
>>>> --------------------- snip --------------------
>>>> library("tcltk")
>>>> tt <- tktoplevel()
>>>> label.widget <- tklabel(tt, text = "Hello, World!")
>>>> button.widget <- tkbutton(tt, text = "Push",
>>>>        command = function()cat("OW!\n"))
>>>> tkpack(label.widget, button.widget) # geometry manager
>>>> --------------------- snip --------------------
>>>
>>>
>>> I can reproduce this. The backtrace shows the crash happens in
>>> dotTclObjv  [/src/library/tcltk/src/tcltk.c@243 ]. This looks like a
>>> bug that was introduced by commit 78408/78409 about a month ago. I
>>> think the problem is that this commit changes 'calloc' to 'Calloc'
>>> without changing the corresponding 'free' to 'Free'.
>>>
>>> This has nothing to do with the Windows build or installation. Nothing
>>> has changed in the windows build procedure between 4.0.0 and 4.0.1.
>>>
>>> ______________________________________________
>>> [hidden email] mailing list
>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>
>>
>
> --
> Luke Tierney
> Ralph E. Wareham Professor of Mathematical Sciences
> University of Iowa                  Phone:             319-335-3386
> Department of Statistics and        Fax:               319-335-3017
>   Actuarial Science
> 241 Schaeffer Hall                  email:   [hidden email]
> Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu

--
Peter Dalgaard, Professor,
Center for Statistics, Copenhagen Business School
Solbjerg Plads 3, 2000 Frederiksberg, Denmark
Phone: (+45)38153501
Office: A 4.23
Email: [hidden email]  Priv: [hidden email]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [External] Re: use of the tcltk package crashes R 4.0.1 for Windows

Peter Dalgaard-2
In reply to this post by Jeroen Ooms


> On 7 Jun 2020, at 18:59 , Jeroen Ooms <[hidden email]> wrote:
>
> On Sun, Jun 7, 2020 at 5:53 PM <[hidden email]> wrote:
>>
>> On Sun, 7 Jun 2020, peter dalgaard wrote:
>>
>>> So this wasn't tested for a month?
>>>
>>> Anyways, Free() is just free() with a check that we're not freeing a null pointer, followed by setting the pointer to NULL. At that point of tcltk.c, we have
>>>
>>>  for (objc = i = 0; i < length(avec); i++){
>>>       const char *s;
>>>       char *tmp;
>>>       if (!isNull(nm) && strlen(s = translateChar(STRING_ELT(nm, i)))){
>>>           //  tmp = calloc(strlen(s)+2, sizeof(char));
>>>           tmp = Calloc(strlen(s)+2, char);
>>>           *tmp = '-';
>>>           strcpy(tmp+1, s);
>>>           objv[objc++] = Tcl_NewStringObj(tmp, -1);
>>>           free(tmp);
>>>       }
>>>       if (!isNull(t = VECTOR_ELT(avec, i)))
>>>           objv[objc++] = (Tcl_Obj *) R_ExternalPtrAddr(t);
>>>   }
>>>
>>> and I can't see how tmp can be NULL at the free(), nor can I see it mattering if it is not set to NULL (notice that it goes out of scope with the for loop).
>>
>> Right. And the calloc->Calloc change doesn't look like an issue either
>> -- just checking for a NULL.
>>
>> If the crash is happening in free() then that most likely means
>> corrupted malloc data structures. Unfortunately that could be
>> happening anywhere.
>
> Writing R extensions, section 6.1.2 says: "Do not assume that memory
> allocated by Calloc/Realloc comes from the same pool as used by
> malloc: in particular do not use free or strdup with it."
>
> I think the reason is that R uses dlmalloc for Calloc on Windows:
> https://github.com/wch/r-source/blob/c634fec5214e73747b44d7c0e6f047fefe44667d/src/main/memory.c#L94-L103

But that section #defines calloc and free to Rm_... counterparts in lockstep? (I assume that is where dlmalloc comes in?)

Anyways, does it actually work to change free() to Free()? If so, then all this post mortem analysis is rather a moot point.

-pd

--
Peter Dalgaard, Professor,
Center for Statistics, Copenhagen Business School
Solbjerg Plads 3, 2000 Frederiksberg, Denmark
Phone: (+45)38153501
Office: A 4.23
Email: [hidden email]  Priv: [hidden email]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [External] Re: use of the tcltk package crashes R 4.0.1 for Windows

Gábor Csárdi
FWIW, you can "test" this by using the daily R builds. They are here:
https://ci.appveyor.com/project/jeroen/base/history

This is the build before the change, it does not crash:
https://ci.appveyor.com/project/jeroen/base/builds/32781690

This is the build right after the change, it does crash:
https://ci.appveyor.com/project/jeroen/base/builds/32808414

(To get the installers, click on "Artifacts" in the top right corner,
download the zip file, it has an exe installer.)

There are a handful of commits between the two installers, but only
this change seems to be related to the crash:
https://github.com/wch/r-source/commit/ba76508c8041335c1896df491ac227fdde0cfb0d
https://github.com/wch/r-source/commit/2c047b94bfe0996419f8871ed6b62b1e7d5ec7bd
https://github.com/wch/r-source/commit/59840c37eb20e05241fb9e85228331fa31db9a2b
https://github.com/wch/r-source/commit/161fc3f703e46201299e87bf7717a93d13c23970
https://github.com/wch/r-source/commit/4c267df39d776fa10c4b2d6b3872dbb85b073681
https://github.com/wch/r-source/commit/f3de035e12a8c8920772297405ed25ee6b3ec4a6

Gabor

On Sun, Jun 7, 2020 at 6:40 PM peter dalgaard <[hidden email]> wrote:

>
>
>
> > On 7 Jun 2020, at 18:59 , Jeroen Ooms <[hidden email]> wrote:
> >
> > On Sun, Jun 7, 2020 at 5:53 PM <[hidden email]> wrote:
> >>
> >> On Sun, 7 Jun 2020, peter dalgaard wrote:
> >>
> >>> So this wasn't tested for a month?
> >>>
> >>> Anyways, Free() is just free() with a check that we're not freeing a null pointer, followed by setting the pointer to NULL. At that point of tcltk.c, we have
> >>>
> >>>  for (objc = i = 0; i < length(avec); i++){
> >>>       const char *s;
> >>>       char *tmp;
> >>>       if (!isNull(nm) && strlen(s = translateChar(STRING_ELT(nm, i)))){
> >>>           //  tmp = calloc(strlen(s)+2, sizeof(char));
> >>>           tmp = Calloc(strlen(s)+2, char);
> >>>           *tmp = '-';
> >>>           strcpy(tmp+1, s);
> >>>           objv[objc++] = Tcl_NewStringObj(tmp, -1);
> >>>           free(tmp);
> >>>       }
> >>>       if (!isNull(t = VECTOR_ELT(avec, i)))
> >>>           objv[objc++] = (Tcl_Obj *) R_ExternalPtrAddr(t);
> >>>   }
> >>>
> >>> and I can't see how tmp can be NULL at the free(), nor can I see it mattering if it is not set to NULL (notice that it goes out of scope with the for loop).
> >>
> >> Right. And the calloc->Calloc change doesn't look like an issue either
> >> -- just checking for a NULL.
> >>
> >> If the crash is happening in free() then that most likely means
> >> corrupted malloc data structures. Unfortunately that could be
> >> happening anywhere.
> >
> > Writing R extensions, section 6.1.2 says: "Do not assume that memory
> > allocated by Calloc/Realloc comes from the same pool as used by
> > malloc: in particular do not use free or strdup with it."
> >
> > I think the reason is that R uses dlmalloc for Calloc on Windows:
> > https://github.com/wch/r-source/blob/c634fec5214e73747b44d7c0e6f047fefe44667d/src/main/memory.c#L94-L103
>
> But that section #defines calloc and free to Rm_... counterparts in lockstep? (I assume that is where dlmalloc comes in?)
>
> Anyways, does it actually work to change free() to Free()? If so, then all this post mortem analysis is rather a moot point.
>
> -pd
>
> --
> Peter Dalgaard, Professor,
> Center for Statistics, Copenhagen Business School
> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
> Phone: (+45)38153501
> Office: A 4.23
> Email: [hidden email]  Priv: [hidden email]
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [External] Re: use of the tcltk package crashes R 4.0.1 for Windows

luke-tierney
In reply to this post by Peter Dalgaard-2
I've committed the change to use Free instead of free in tcltk.c and
sys-std.c (r78652 for R-devel, r78653 for R-patched).

We might consider either moving Calloc/Free out of the Windows
remapping or moving the remapping into header files so everything
seeing our header files uses our calloc/free. Either would be less
brittle that the current status.

Best,

luke

On Sun, 7 Jun 2020, peter dalgaard wrote:

>
>
>> On 7 Jun 2020, at 18:59 , Jeroen Ooms <[hidden email]> wrote:
>>
>> On Sun, Jun 7, 2020 at 5:53 PM <[hidden email]> wrote:
>>>
>>> On Sun, 7 Jun 2020, peter dalgaard wrote:
>>>
>>>> So this wasn't tested for a month?
>>>>
>>>> Anyways, Free() is just free() with a check that we're not freeing a null pointer, followed by setting the pointer to NULL. At that point of tcltk.c, we have
>>>>
>>>>  for (objc = i = 0; i < length(avec); i++){
>>>>       const char *s;
>>>>       char *tmp;
>>>>       if (!isNull(nm) && strlen(s = translateChar(STRING_ELT(nm, i)))){
>>>>           //  tmp = calloc(strlen(s)+2, sizeof(char));
>>>>           tmp = Calloc(strlen(s)+2, char);
>>>>           *tmp = '-';
>>>>           strcpy(tmp+1, s);
>>>>           objv[objc++] = Tcl_NewStringObj(tmp, -1);
>>>>           free(tmp);
>>>>       }
>>>>       if (!isNull(t = VECTOR_ELT(avec, i)))
>>>>           objv[objc++] = (Tcl_Obj *) R_ExternalPtrAddr(t);
>>>>   }
>>>>
>>>> and I can't see how tmp can be NULL at the free(), nor can I see it mattering if it is not set to NULL (notice that it goes out of scope with the for loop).
>>>
>>> Right. And the calloc->Calloc change doesn't look like an issue either
>>> -- just checking for a NULL.
>>>
>>> If the crash is happening in free() then that most likely means
>>> corrupted malloc data structures. Unfortunately that could be
>>> happening anywhere.
>>
>> Writing R extensions, section 6.1.2 says: "Do not assume that memory
>> allocated by Calloc/Realloc comes from the same pool as used by
>> malloc: in particular do not use free or strdup with it."
>>
>> I think the reason is that R uses dlmalloc for Calloc on Windows:
>> https://github.com/wch/r-source/blob/c634fec5214e73747b44d7c0e6f047fefe44667d/src/main/memory.c#L94-L103
>
> But that section #defines calloc and free to Rm_... counterparts in lockstep? (I assume that is where dlmalloc comes in?)
>
> Anyways, does it actually work to change free() to Free()? If so, then all this post mortem analysis is rather a moot point.
>
> -pd
>
>

--
Luke Tierney
Ralph E. Wareham Professor of Mathematical Sciences
University of Iowa                  Phone:             319-335-3386
Department of Statistics and        Fax:               319-335-3017
    Actuarial Science
241 Schaeffer Hall                  email:   [hidden email]
Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [External] use of the tcltk package crashes R 4.0.1 for Windows

Peter Dalgaard-2
Ah, I see it now:

The remapping of free() to Rm_free() and calloc() to Rm_calloc() happens in memory.c, but not in tcltk.c; the macro Calloc in R_ext/RS.h maps to a call to R_chk_alloc which is defined in memory.h; RS.h is included in tcltk.c, so tcltk.c winds up calling Rm_calloc() via Calloc(), but then the NON-remapped free(), and the walls come tumbling down.

If the  "#if defined(Win32)" block had been inside RS.h, the problem wouldn't arise.

-pd

> On 8 Jun 2020, at 00:03 , [hidden email] wrote:
>
> I've committed the change to use Free instead of free in tcltk.c and
> sys-std.c (r78652 for R-devel, r78653 for R-patched).
>
> We might consider either moving Calloc/Free out of the Windows
> remapping or moving the remapping into header files so everything
> seeing our header files uses our calloc/free. Either would be less
> brittle that the current status.
>
> Best,
>
> luke
>
> On Sun, 7 Jun 2020, peter dalgaard wrote:
>
>>
>>
>>> On 7 Jun 2020, at 18:59 , Jeroen Ooms <[hidden email]> wrote:
>>>
>>> On Sun, Jun 7, 2020 at 5:53 PM <[hidden email]> wrote:
>>>>
>>>> On Sun, 7 Jun 2020, peter dalgaard wrote:
>>>>
>>>>> So this wasn't tested for a month?
>>>>>
>>>>> Anyways, Free() is just free() with a check that we're not freeing a null pointer, followed by setting the pointer to NULL. At that point of tcltk.c, we have
>>>>>
>>>>> for (objc = i = 0; i < length(avec); i++){
>>>>>      const char *s;
>>>>>      char *tmp;
>>>>>      if (!isNull(nm) && strlen(s = translateChar(STRING_ELT(nm, i)))){
>>>>>          //  tmp = calloc(strlen(s)+2, sizeof(char));
>>>>>          tmp = Calloc(strlen(s)+2, char);
>>>>>          *tmp = '-';
>>>>>          strcpy(tmp+1, s);
>>>>>          objv[objc++] = Tcl_NewStringObj(tmp, -1);
>>>>>          free(tmp);
>>>>>      }
>>>>>      if (!isNull(t = VECTOR_ELT(avec, i)))
>>>>>          objv[objc++] = (Tcl_Obj *) R_ExternalPtrAddr(t);
>>>>>  }
>>>>>
>>>>> and I can't see how tmp can be NULL at the free(), nor can I see it mattering if it is not set to NULL (notice that it goes out of scope with the for loop).
>>>>
>>>> Right. And the calloc->Calloc change doesn't look like an issue either
>>>> -- just checking for a NULL.
>>>>
>>>> If the crash is happening in free() then that most likely means
>>>> corrupted malloc data structures. Unfortunately that could be
>>>> happening anywhere.
>>>
>>> Writing R extensions, section 6.1.2 says: "Do not assume that memory
>>> allocated by Calloc/Realloc comes from the same pool as used by
>>> malloc: in particular do not use free or strdup with it."
>>>
>>> I think the reason is that R uses dlmalloc for Calloc on Windows:
>>> https://github.com/wch/r-source/blob/c634fec5214e73747b44d7c0e6f047fefe44667d/src/main/memory.c#L94-L103
>>
>> But that section #defines calloc and free to Rm_... counterparts in lockstep? (I assume that is where dlmalloc comes in?)
>>
>> Anyways, does it actually work to change free() to Free()? If so, then all this post mortem analysis is rather a moot point.
>>
>> -pd
>>
>>
>
> --
> Luke Tierney
> Ralph E. Wareham Professor of Mathematical Sciences
> University of Iowa                  Phone:             319-335-3386
> Department of Statistics and        Fax:               319-335-3017
>   Actuarial Science
> 241 Schaeffer Hall                  email:   [hidden email]
> Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu

--
Peter Dalgaard, Professor,
Center for Statistics, Copenhagen Business School
Solbjerg Plads 3, 2000 Frederiksberg, Denmark
Phone: (+45)38153501
Office: A 4.23
Email: [hidden email]  Priv: [hidden email]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [External] Re: use of the tcltk package crashes R 4.0.1 for Windows

Abby Spurdle
In reply to this post by Fox, John
On Mon, Jun 8, 2020 at 4:09 AM Fox, John <[hidden email]> wrote:
> Does it make sense to withdraw the Windows R 4.0.1 binary until the issue is resolved?

Yes, it does.
All the release reversions should be removed.

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [External] Re: use of the tcltk package crashes R 4.0.1 for Windows

Abby Spurdle
sorry, release "versions"


On Mon, Jun 8, 2020 at 11:17 AM Abby Spurdle <[hidden email]> wrote:
>
> On Mon, Jun 8, 2020 at 4:09 AM Fox, John <[hidden email]> wrote:
> > Does it make sense to withdraw the Windows R 4.0.1 binary until the issue is resolved?
>
> Yes, it does.
> All the release reversions should be removed.

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
12