Something is wrong with the unserialize function

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Something is wrong with the unserialize function

Jiefei Wang
Hi all,

I am not able to export an ALTREP object when `gctorture` is on in the
worker. The package simplemmap can be used to reproduce the problem. See
the example below
```
## Create a temporary file
filePath <- tempfile()
con <- file(filePath, "wrb")
writeBin(rep(0.0,10),con)
close(con)

library(simplemmap)
library(parallel)
cl <- makeCluster(1)
x <- mmap(filePath, "double")
## Turn gctorture on
clusterEvalQ(cl, gctorture())
clusterExport(cl, "x")
## x is an 0-length vector on the worker
clusterEvalQ(cl, x)
stopCluster(cl)
```

you can find more info on the problem if you manually build a connection
between two R processes and export the ALTREP object. See output below
```
> con <- socketConnection(port = 1234,server = FALSE)
> gctorture()
> x <- unserialize(con)
Warning message:
In unserialize(con) :
  cannot unserialize ALTVEC object of class 'mmap_real' from package
'simplemmap'; returning length zero vector
```
It seems like  simplemmap did not get loaded correctly on the worker. If
you run `library( simplemmap)` before unserializing the ALTREP, there will
be no problem. But I suppose we should be able to unserialize objects
without preloading the library?

This issue can be reproduced on Ubuntu with R version 4.0.2 (2020-06-22)
and Windows with R Under development (unstable) (2020-09-03 r79126).

Here is the link to simplemmap:
https://github.com/ALTREP-examples/Rpkg-simplemmap

Best,
Jiefei

        [[alternative HTML version deleted]]

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

Re: [External] Something is wrong with the unserialize function

luke-tierney
Thanks for the report. Will look into it when I get a chance unless
someone else gets there first.

A simpler reprex:

## create and serialize a memmory-mapped file object
filePath <- "x.dat"
con <- file(filePath, "wrb")
writeBin(rep(0.0,10),con)
close(con)

library(simplemmap)
x <- mmap(filePath, "double")
saveRDS(x, file = "x.Rds")

## in a separate R process:
gctorture()
readRDS("x.Rds")

Looks like a missing PROTECT somewhere.

Best,

luke

On Thu, 29 Oct 2020, Jiefei Wang wrote:

> Hi all,
>
> I am not able to export an ALTREP object when `gctorture` is on in the
> worker. The package simplemmap can be used to reproduce the problem. See
> the example below
> ```
> ## Create a temporary file
> filePath <- tempfile()
> con <- file(filePath, "wrb")
> writeBin(rep(0.0,10),con)
> close(con)
>
> library(simplemmap)
> library(parallel)
> cl <- makeCluster(1)
> x <- mmap(filePath, "double")
> ## Turn gctorture on
> clusterEvalQ(cl, gctorture())
> clusterExport(cl, "x")
> ## x is an 0-length vector on the worker
> clusterEvalQ(cl, x)
> stopCluster(cl)
> ```
>
> you can find more info on the problem if you manually build a connection
> between two R processes and export the ALTREP object. See output below
> ```
>> con <- socketConnection(port = 1234,server = FALSE)
>> gctorture()
>> x <- unserialize(con)
> Warning message:
> In unserialize(con) :
>  cannot unserialize ALTVEC object of class 'mmap_real' from package
> 'simplemmap'; returning length zero vector
> ```
> It seems like  simplemmap did not get loaded correctly on the worker. If
> you run `library( simplemmap)` before unserializing the ALTREP, there will
> be no problem. But I suppose we should be able to unserialize objects
> without preloading the library?
>
> This issue can be reproduced on Ubuntu with R version 4.0.2 (2020-06-22)
> and Windows with R Under development (unstable) (2020-09-03 r79126).
>
> Here is the link to simplemmap:
> https://github.com/ALTREP-examples/Rpkg-simplemmap
>
> Best,
> Jiefei
>
> [[alternative HTML version deleted]]
>
> ______________________________________________
> [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] Something is wrong with the unserialize function

Martin Morgan-4
This

Index: src/main/altrep.c
===================================================================
--- src/main/altrep.c (revision 79385)
+++ src/main/altrep.c (working copy)
@@ -275,10 +275,11 @@
  SEXP psym = ALTREP_SERIALIZED_CLASS_PKGSYM(info);
  SEXP class = LookupClass(csym, psym);
  if (class == NULL) {
-    SEXP pname = ScalarString(PRINTNAME(psym));
+    SEXP pname = PROTECT(ScalarString(PRINTNAME(psym)));
     R_tryCatchError(find_namespace, pname,
     handle_namespace_error, NULL);
     class = LookupClass(csym, psym);
+    UNPROTECT(1);
  }
  return class;
     }

seems to remove the warning; I'm guessing that the other SEXP already exist so don't need protecting?

Martin Morgan


On 10/29/20, 12:47 PM, "R-devel on behalf of [hidden email]" <[hidden email] on behalf of [hidden email]> wrote:

    Thanks for the report. Will look into it when I get a chance unless
    someone else gets there first.

    A simpler reprex:

    ## create and serialize a memmory-mapped file object
    filePath <- "x.dat"
    con <- file(filePath, "wrb")
    writeBin(rep(0.0,10),con)
    close(con)

    library(simplemmap)
    x <- mmap(filePath, "double")
    saveRDS(x, file = "x.Rds")

    ## in a separate R process:
    gctorture()
    readRDS("x.Rds")

    Looks like a missing PROTECT somewhere.

    Best,

    luke

    On Thu, 29 Oct 2020, Jiefei Wang wrote:

    > Hi all,
    >
    > I am not able to export an ALTREP object when `gctorture` is on in the
    > worker. The package simplemmap can be used to reproduce the problem. See
    > the example below
    > ```
    > ## Create a temporary file
    > filePath <- tempfile()
    > con <- file(filePath, "wrb")
    > writeBin(rep(0.0,10),con)
    > close(con)
    >
    > library(simplemmap)
    > library(parallel)
    > cl <- makeCluster(1)
    > x <- mmap(filePath, "double")
    > ## Turn gctorture on
    > clusterEvalQ(cl, gctorture())
    > clusterExport(cl, "x")
    > ## x is an 0-length vector on the worker
    > clusterEvalQ(cl, x)
    > stopCluster(cl)
    > ```
    >
    > you can find more info on the problem if you manually build a connection
    > between two R processes and export the ALTREP object. See output below
    > ```
    >> con <- socketConnection(port = 1234,server = FALSE)
    >> gctorture()
    >> x <- unserialize(con)
    > Warning message:
    > In unserialize(con) :
    >  cannot unserialize ALTVEC object of class 'mmap_real' from package
    > 'simplemmap'; returning length zero vector
    > ```
    > It seems like  simplemmap did not get loaded correctly on the worker. If
    > you run `library( simplemmap)` before unserializing the ALTREP, there will
    > be no problem. But I suppose we should be able to unserialize objects
    > without preloading the library?
    >
    > This issue can be reproduced on Ubuntu with R version 4.0.2 (2020-06-22)
    > and Windows with R Under development (unstable) (2020-09-03 r79126).
    >
    > Here is the link to simplemmap:
    > https://github.com/ALTREP-examples/Rpkg-simplemmap
    >
    > Best,
    > Jiefei
    >
    > [[alternative HTML version deleted]]
    >
    > ______________________________________________
    > [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
______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [External] Something is wrong with the unserialize function

luke-tierney
I found that also; fixed in r79386 in the trunk. Will port to R-patched
shortly.

Best,

luke

On Thu, 29 Oct 2020, Martin Morgan wrote:

> This
>
> Index: src/main/altrep.c
> ===================================================================
> --- src/main/altrep.c (revision 79385)
> +++ src/main/altrep.c (working copy)
> @@ -275,10 +275,11 @@
> SEXP psym = ALTREP_SERIALIZED_CLASS_PKGSYM(info);
> SEXP class = LookupClass(csym, psym);
> if (class == NULL) {
> -    SEXP pname = ScalarString(PRINTNAME(psym));
> +    SEXP pname = PROTECT(ScalarString(PRINTNAME(psym)));
>    R_tryCatchError(find_namespace, pname,
>    handle_namespace_error, NULL);
>    class = LookupClass(csym, psym);
> +    UNPROTECT(1);
> }
> return class;
>     }
>
> seems to remove the warning; I'm guessing that the other SEXP already exist so don't need protecting?
>
> Martin Morgan
>
>
> On 10/29/20, 12:47 PM, "R-devel on behalf of [hidden email]" <[hidden email] on behalf of [hidden email]> wrote:
>
>    Thanks for the report. Will look into it when I get a chance unless
>    someone else gets there first.
>
>    A simpler reprex:
>
>    ## create and serialize a memmory-mapped file object
>    filePath <- "x.dat"
>    con <- file(filePath, "wrb")
>    writeBin(rep(0.0,10),con)
>    close(con)
>
>    library(simplemmap)
>    x <- mmap(filePath, "double")
>    saveRDS(x, file = "x.Rds")
>
>    ## in a separate R process:
>    gctorture()
>    readRDS("x.Rds")
>
>    Looks like a missing PROTECT somewhere.
>
>    Best,
>
>    luke
>
>    On Thu, 29 Oct 2020, Jiefei Wang wrote:
>
>    > Hi all,
>    >
>    > I am not able to export an ALTREP object when `gctorture` is on in the
>    > worker. The package simplemmap can be used to reproduce the problem. See
>    > the example below
>    > ```
>    > ## Create a temporary file
>    > filePath <- tempfile()
>    > con <- file(filePath, "wrb")
>    > writeBin(rep(0.0,10),con)
>    > close(con)
>    >
>    > library(simplemmap)
>    > library(parallel)
>    > cl <- makeCluster(1)
>    > x <- mmap(filePath, "double")
>    > ## Turn gctorture on
>    > clusterEvalQ(cl, gctorture())
>    > clusterExport(cl, "x")
>    > ## x is an 0-length vector on the worker
>    > clusterEvalQ(cl, x)
>    > stopCluster(cl)
>    > ```
>    >
>    > you can find more info on the problem if you manually build a connection
>    > between two R processes and export the ALTREP object. See output below
>    > ```
>    >> con <- socketConnection(port = 1234,server = FALSE)
>    >> gctorture()
>    >> x <- unserialize(con)
>    > Warning message:
>    > In unserialize(con) :
>    >  cannot unserialize ALTVEC object of class 'mmap_real' from package
>    > 'simplemmap'; returning length zero vector
>    > ```
>    > It seems like  simplemmap did not get loaded correctly on the worker. If
>    > you run `library( simplemmap)` before unserializing the ALTREP, there will
>    > be no problem. But I suppose we should be able to unserialize objects
>    > without preloading the library?
>    >
>    > This issue can be reproduced on Ubuntu with R version 4.0.2 (2020-06-22)
>    > and Windows with R Under development (unstable) (2020-09-03 r79126).
>    >
>    > Here is the link to simplemmap:
>    > https://github.com/ALTREP-examples/Rpkg-simplemmap
>    >
>    > Best,
>    > Jiefei
>    >
>    > [[alternative HTML version deleted]]
>    >
>    > ______________________________________________
>    > [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
>

--
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] Something is wrong with the unserialize function

Jiefei Wang
Thanks for the fix! I will wait for the update and try it again.

Best,
Jiefei

On Fri, Oct 30, 2020 at 1:42 AM <[hidden email]> wrote:

> I found that also; fixed in r79386 in the trunk. Will port to R-patched
> shortly.
>
> Best,
>
> luke
>
> On Thu, 29 Oct 2020, Martin Morgan wrote:
>
> > This
> >
> > Index: src/main/altrep.c
> > ===================================================================
> > --- src/main/altrep.c (revision 79385)
> > +++ src/main/altrep.c (working copy)
> > @@ -275,10 +275,11 @@
> >       SEXP psym = ALTREP_SERIALIZED_CLASS_PKGSYM(info);
> >       SEXP class = LookupClass(csym, psym);
> >       if (class == NULL) {
> > -         SEXP pname = ScalarString(PRINTNAME(psym));
> > +         SEXP pname = PROTECT(ScalarString(PRINTNAME(psym)));
> >           R_tryCatchError(find_namespace, pname,
> >                           handle_namespace_error, NULL);
> >           class = LookupClass(csym, psym);
> > +         UNPROTECT(1);
> >       }
> >       return class;
> >     }
> >
> > seems to remove the warning; I'm guessing that the other SEXP already
> exist so don't need protecting?
> >
> > Martin Morgan
> >
> >
> > On 10/29/20, 12:47 PM, "R-devel on behalf of [hidden email]" <
> [hidden email] on behalf of [hidden email]> wrote:
> >
> >    Thanks for the report. Will look into it when I get a chance unless
> >    someone else gets there first.
> >
> >    A simpler reprex:
> >
> >    ## create and serialize a memmory-mapped file object
> >    filePath <- "x.dat"
> >    con <- file(filePath, "wrb")
> >    writeBin(rep(0.0,10),con)
> >    close(con)
> >
> >    library(simplemmap)
> >    x <- mmap(filePath, "double")
> >    saveRDS(x, file = "x.Rds")
> >
> >    ## in a separate R process:
> >    gctorture()
> >    readRDS("x.Rds")
> >
> >    Looks like a missing PROTECT somewhere.
> >
> >    Best,
> >
> >    luke
> >
> >    On Thu, 29 Oct 2020, Jiefei Wang wrote:
> >
> >    > Hi all,
> >    >
> >    > I am not able to export an ALTREP object when `gctorture` is on in
> the
> >    > worker. The package simplemmap can be used to reproduce the
> problem. See
> >    > the example below
> >    > ```
> >    > ## Create a temporary file
> >    > filePath <- tempfile()
> >    > con <- file(filePath, "wrb")
> >    > writeBin(rep(0.0,10),con)
> >    > close(con)
> >    >
> >    > library(simplemmap)
> >    > library(parallel)
> >    > cl <- makeCluster(1)
> >    > x <- mmap(filePath, "double")
> >    > ## Turn gctorture on
> >    > clusterEvalQ(cl, gctorture())
> >    > clusterExport(cl, "x")
> >    > ## x is an 0-length vector on the worker
> >    > clusterEvalQ(cl, x)
> >    > stopCluster(cl)
> >    > ```
> >    >
> >    > you can find more info on the problem if you manually build a
> connection
> >    > between two R processes and export the ALTREP object. See output
> below
> >    > ```
> >    >> con <- socketConnection(port = 1234,server = FALSE)
> >    >> gctorture()
> >    >> x <- unserialize(con)
> >    > Warning message:
> >    > In unserialize(con) :
> >    >  cannot unserialize ALTVEC object of class 'mmap_real' from package
> >    > 'simplemmap'; returning length zero vector
> >    > ```
> >    > It seems like  simplemmap did not get loaded correctly on the
> worker. If
> >    > you run `library( simplemmap)` before unserializing the ALTREP,
> there will
> >    > be no problem. But I suppose we should be able to unserialize
> objects
> >    > without preloading the library?
> >    >
> >    > This issue can be reproduced on Ubuntu with R version 4.0.2
> (2020-06-22)
> >    > and Windows with R Under development (unstable) (2020-09-03 r79126).
> >    >
> >    > Here is the link to simplemmap:
> >    > https://github.com/ALTREP-examples/Rpkg-simplemmap
> >    >
> >    > Best,
> >    > Jiefei
> >    >
> >    >  [[alternative HTML version deleted]]
> >    >
> >    > ______________________________________________
> >    > [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
> >
>
> --
> 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

        [[alternative HTML version deleted]]

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