check does not check that package examples remove tempdir()

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

check does not check that package examples remove tempdir()

R devel mailing list
I was looking at the CRAN package 'bfork-0.1.2', which exposes the Unix
fork() and waitpid() calls at the R code level, and noticed that the help
file example for bfork::fork removes R's temporary directory, the value of
tempdir().   I think it happens because the forked process shares the value
of tempdir() with the parent process and removes it when it exits.

This seems like a serious problem - should 'check' make sure that running
code in a package's examples, vignettes, etc. leaves tempdir() intact?

> dir.exists(tempdir())
[1] TRUE
> library(bfork)
> example(fork)

fork>     ## create a function to be run as a separate process
fork>     fn <- function() {
fork+         Sys.sleep(4)
fork+         print("World!")
fork+     }

fork>     ## fork the process
fork>     pid <- fork(fn)

fork>     ## do work in the parent process
fork>     print("Hello")
[1] "Hello"

fork>     ## wait for the child process
fork>     waitpid(pid)
[1] "World!"
[1] 7063
> dir.exists(tempdir())
[1] FALSE


Bill Dunlap
TIBCO Software
wdunlap tibco.com

        [[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: check does not check that package examples remove tempdir()

Henrik Bengtsson-5
Related to this problem - from R-devel NEWS
(https://cran.r-project.org/doc/manuals/r-devel/NEWS.html):

* tempdir(check = TRUE) recreates the tmpdir() if it is no longer
valid (e.g. because some other process has cleaned up the ‘/tmp’
directory).

Not sure if there's a plan to make check = TRUE the default though.

/Henrik

On Wed, Nov 8, 2017 at 4:43 PM, William Dunlap via R-devel
<[hidden email]> wrote:

> I was looking at the CRAN package 'bfork-0.1.2', which exposes the Unix
> fork() and waitpid() calls at the R code level, and noticed that the help
> file example for bfork::fork removes R's temporary directory, the value of
> tempdir().   I think it happens because the forked process shares the value
> of tempdir() with the parent process and removes it when it exits.
>
> This seems like a serious problem - should 'check' make sure that running
> code in a package's examples, vignettes, etc. leaves tempdir() intact?
>
>> dir.exists(tempdir())
> [1] TRUE
>> library(bfork)
>> example(fork)
>
> fork>     ## create a function to be run as a separate process
> fork>     fn <- function() {
> fork+         Sys.sleep(4)
> fork+         print("World!")
> fork+     }
>
> fork>     ## fork the process
> fork>     pid <- fork(fn)
>
> fork>     ## do work in the parent process
> fork>     print("Hello")
> [1] "Hello"
>
> fork>     ## wait for the child process
> fork>     waitpid(pid)
> [1] "World!"
> [1] 7063
>> dir.exists(tempdir())
> [1] FALSE
>
>
> Bill Dunlap
> TIBCO Software
> wdunlap tibco.com
>
>         [[alternative HTML version deleted]]
>
> ______________________________________________
> [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: check does not check that package examples remove tempdir()

R devel mailing list
I think recreating tempdir() is ok in an emergency situation, but package
code
should not be removing tempdir() - it may contain important information.

Bill Dunlap
TIBCO Software
wdunlap tibco.com

On Wed, Nov 8, 2017 at 4:55 PM, Henrik Bengtsson <[hidden email]
> wrote:

> Related to this problem - from R-devel NEWS
> (https://cran.r-project.org/doc/manuals/r-devel/NEWS.html):
>
> * tempdir(check = TRUE) recreates the tmpdir() if it is no longer
> valid (e.g. because some other process has cleaned up the ‘/tmp’
> directory).
>
> Not sure if there's a plan to make check = TRUE the default though.
>
> /Henrik
>
> On Wed, Nov 8, 2017 at 4:43 PM, William Dunlap via R-devel
> <[hidden email]> wrote:
> > I was looking at the CRAN package 'bfork-0.1.2', which exposes the Unix
> > fork() and waitpid() calls at the R code level, and noticed that the help
> > file example for bfork::fork removes R's temporary directory, the value
> of
> > tempdir().   I think it happens because the forked process shares the
> value
> > of tempdir() with the parent process and removes it when it exits.
> >
> > This seems like a serious problem - should 'check' make sure that running
> > code in a package's examples, vignettes, etc. leaves tempdir() intact?
> >
> >> dir.exists(tempdir())
> > [1] TRUE
> >> library(bfork)
> >> example(fork)
> >
> > fork>     ## create a function to be run as a separate process
> > fork>     fn <- function() {
> > fork+         Sys.sleep(4)
> > fork+         print("World!")
> > fork+     }
> >
> > fork>     ## fork the process
> > fork>     pid <- fork(fn)
> >
> > fork>     ## do work in the parent process
> > fork>     print("Hello")
> > [1] "Hello"
> >
> > fork>     ## wait for the child process
> > fork>     waitpid(pid)
> > [1] "World!"
> > [1] 7063
> >> dir.exists(tempdir())
> > [1] FALSE
> >
> >
> > Bill Dunlap
> > TIBCO Software
> > wdunlap tibco.com
> >
> >         [[alternative HTML version deleted]]
> >
> > ______________________________________________
> > [hidden email] mailing list
> > https://stat.ethz.ch/mailman/listinfo/r-devel
>

        [[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: check does not check that package examples remove tempdir()

danlrobertson89
> tempdir(). I think it happens because the forked process shares the
> value of tempdir() with the parent process and removes it when it
> exits.

This is very likely the case. Pretty much the entire library can be
summed up by bfork_fork, which is the following.

    SEXP res;
    pid_t pid;
    if((pid = fork()) == 0) {
        PROTECT(res = eval(lang1(fn), R_GlobalEnv));
        PROTECT(res = eval(lang2(install("q"), mkString("no")), R_GlobalEnv));
        UNPROTECT(2);
    }

    return ScalarInteger(pid);

I wrote this lib when I was still in school and can see several issues
with the implementation of `bfork_fork`. This issue happens because
we do not exit with _exit, but by essentially calling q("no").

Cheers,

Dan

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

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: check does not check that package examples remove tempdir()

Tomas Kalibera

Please note there is parallel::mcparallel/mccollect in R which provides
similar functionality, mcparallel starts a new job and mccollect allows
to wait for it.

You are right about _exit, but there are additional issues which cannot
be solved independently in an external package, and, such a low level
interface cannot be used without race conditions from R anyway.

Best
Tomas

On 11/09/2017 02:55 AM, [hidden email] wrote:

>> tempdir(). I think it happens because the forked process shares the
>> value of tempdir() with the parent process and removes it when it
>> exits.
> This is very likely the case. Pretty much the entire library can be
> summed up by bfork_fork, which is the following.
>
>      SEXP res;
>      pid_t pid;
>      if((pid = fork()) == 0) {
>          PROTECT(res = eval(lang1(fn), R_GlobalEnv));
>          PROTECT(res = eval(lang2(install("q"), mkString("no")), R_GlobalEnv));
>          UNPROTECT(2);
>      }
>
>      return ScalarInteger(pid);
>
> I wrote this lib when I was still in school and can see several issues
> with the implementation of `bfork_fork`. This issue happens because
> we do not exit with _exit, but by essentially calling q("no").
>
> Cheers,
>
> Dan
>
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel



        [[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: check does not check that package examples remove tempdir()

Jeroen Ooms
In reply to this post by R devel mailing list
On Thu, Nov 9, 2017 at 1:43 AM, William Dunlap via R-devel
<[hidden email]> wrote:
> I was looking at the CRAN package 'bfork-0.1.2', which exposes the Unix
> fork() and waitpid() calls at the R code level, and noticed that the help
> file example for bfork::fork removes R's temporary directory, the value of
> tempdir().   I think it happens because the forked process shares the value
> of tempdir() with the parent process and removes it when it exits.

This has come up a few times:

 - https://stat.ethz.ch/pipermail/r-devel/2017-February/073748.html
 - https://stat.ethz.ch/pipermail/r-devel/2017-April/074149.html

You may want to read up on the latter discussion.

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