Bug 17156 - parallel::newPSOCknode sends outfile arg unquoted, leading to odd behaviour
Summary: parallel::newPSOCknode sends outfile arg unquoted, leading to odd behaviour
Status: UNCONFIRMED
Alias: None
Product: R
Classification: Unclassified
Component: Low-level (show other bugs)
Version: R-devel (trunk)
Hardware: All All
: P5 minor
Assignee: R-core
URL:
Depends on:
Blocks:
 
Reported: 2016-09-21 05:30 UTC by Andrew Christianson
Modified: 2016-09-26 15:07 UTC (History)
3 users (show)

See Also:


Attachments
Quotes OUT (557 bytes, patch)
2016-09-26 05:44 UTC, Henrik Bengtsson
Details | Diff
shQuote:d OUT (558 bytes, patch)
2016-09-26 15:04 UTC, Henrik Bengtsson
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Christianson 2016-09-21 05:30:12 UTC
If the arg outfile to parallel::makeCluster(type="PSOCK") is a path and contains spaces, odd behavior results.  Either the log file is mistakenly placed at the substring of the path up to the first space (if that doesn't exist, or is writeable), or makeCluster fails and hangs (if is does, and isn't).

Example:

# misplaced
cl1 <- parallel::makeCluster(1:2, type="PSOCK", outfile=file.path(getwd(), "testing space", "test.log"))

#hangs
dir.create("testing2")
cl1 <- parallel::makeCluster(1:2, type="PSOCK", outfile=file.path(getwd(), "testing2 space", "test.log"))

This can be mitigated by shell quoting the outfile.  However, seems like this should either be documented or parallel::newPSOCKnode should shQuote the outfile option (src/library/parallel/R/snowSOCK.R#42). Slightly tangentially, another potential hang here can be mitigated by checking that dirname(outfile) exists.
Comment 1 Henrik Bengtsson 2016-09-26 05:44:37 UTC
Created attachment 2165 [details]
Quotes OUT
Comment 2 Henrik Bengtsson 2016-09-26 05:45:23 UTC
This illustrates what Andrew says:

> trace(system, tracer=quote(message("Command: ", command)), print=FALSE)
Tracing function "system" in package "base"
[1] "system"

## OUT is now quoted:
> cl1 <- parallel::makeCluster(1:2, type="PSOCK", outfile=file.path(getwd(), "testing space", "test.log"))
Command: '/usr/lib/R/bin/Rscript' --default-packages=datasets,utils,grDevices,graphics,stats,methods -e 'parallel:::.slaveRSOCK()' MASTER=localhost PORT=11838 OUT=/home/hb/testing space/test.log TIMEOUT=2592000 XDR=TRUE
[...]

## Manual quoting:
> cl1 <- parallel::makeCluster(1:2, type="PSOCK", outfile=sQuote(file.path(getwd(), "testing space", "test.log")))
Command: '/usr/lib/R/bin/Rscript' --default-packages=datasets,utils,grDevices,graphics,stats,methods -e 'parallel:::.slaveRSOCK()' MASTER=localhost PORT=11838 OUT=‘/home/hb/testing space/test.log’ TIMEOUT=2592000 XDR=TRUE
[...]


Suggested patch (also attached):

svn diff src/library/parallel/R/snowSOCK.R
Index: src/library/parallel/R/snowSOCK.R
===================================================================
--- src/library/parallel/R/snowSOCK.R	(revision 71358)
+++ src/library/parallel/R/snowSOCK.R	(working copy)
@@ -39,7 +39,7 @@
     ## build the local command for starting the worker
     env <- paste0("MASTER=", master,
                  " PORT=", port,
-                 " OUT=", outfile,
+                 " OUT=", sQuote(outfile),
                  " TIMEOUT=", timeout,
                  " XDR=", useXDR)
     arg <- "parallel:::.slaveRSOCK()"
Comment 3 Duncan Murdoch 2016-09-26 08:15:48 UTC
That should be shQuote() rather than sQuote, shouldn't it?
Comment 4 Henrik Bengtsson 2016-09-26 15:04:01 UTC
Created attachment 2166 [details]
shQuote:d OUT
Comment 5 Henrik Bengtsson 2016-09-26 15:06:04 UTC
Duncan, yes, should have been shQuote(). See new patch (also attached):

Index: src/library/parallel/R/snowSOCK.R
===================================================================
--- src/library/parallel/R/snowSOCK.R	(revision 71365)
+++ src/library/parallel/R/snowSOCK.R	(working copy)
@@ -39,7 +39,7 @@
     ## build the local command for starting the worker
     env <- paste0("MASTER=", master,
                  " PORT=", port,
-                 " OUT=", outfile,
+                 " OUT=", shQuote(outfile),
                  " TIMEOUT=", timeout,
                  " XDR=", useXDR)
     arg <- "parallel:::.slaveRSOCK()"

This gives (in rebuilt R-devel):

> trace(system, tracer=quote(message("Command: ", command)), print=FALSE)
Tracing function "system" in package "base"
[1] "system"
> cl1 <- parallel::makeCluster(1:2, type="PSOCK", outfile=file.path(getwd(), "testing space", "test.log"))
Command: '/usr/lib/R/bin/Rscript' --default-packages=datasets,utils,grDevices,graphics,stats,methods -e 'parallel:::.slaveRSOCK()' MASTER=localhost PORT=11774 OUT=/home/hb/repositories/others/r-source-svn/testing space/test.log TIMEOUT=2592000 XDR=TRUE
Comment 6 Henrik Bengtsson 2016-09-26 15:07:08 UTC
Ah... here's the R-devel output:

> trace(system, tracer=quote(message("Command: ", command)), print=FALSE)
Tracing function "system" in package "base"
[1] "system"
> cl1 <- parallel::makeCluster(1:2, type="PSOCK", outfile=file.path(getwd(), "testing space", "test.log"))
Command: '/home/hb/software/R/lib/R/bin/Rscript' --default-packages=datasets,utils,grDevices,graphics,stats,methods -e 'parallel:::.slaveRSOCK()' MASTER=localhost PORT=11541 OUT='/home/hb/repositories/others/r-source-svn/testing space/test.log' TIMEOUT=2592000 XDR=TRUE