lbartnik / subprocess

Other
49 stars 10 forks source link

Is timeout working as expected? #32

Closed johndharrison closed 7 years ago

johndharrison commented 7 years ago
library(subprocess)
proc1 <- tempfile(fileext = ".sh")
cat("#!/bin/sh\n", file = proc1)
cat("while true; do\n", file = proc1, append = TRUE)
cat("(>&2 echo \"error\")\n", file = proc1, append = TRUE)
cat("sleep 3\n", file = proc1, append = TRUE)
cat("done\n", file = proc1, append = TRUE)

Sys.chmod(proc1, "700")

handle1 <- spawn_process(proc1)
for(i in 1:10){
  print(process_read(handle1, "stderr", timeout = 1000))
}
process_kill(handle1)

Running the above i get:

> for(i in 1:10){
+   print(process_read(handle1, "stderr", timeout = 1000))
+ }
[1] "error"
Error in process_read(handle1, "stderr", timeout = 1000) : 
  error while reading from child process

I had expected subprocess to check for output to stderr for 1 second if none found return character(0).

lbartnik commented 7 years ago

Yes, AFAIR you're right. I've been working on an alternative read code where one can wait on both output streams, it's in a separate branch now (wait-for-both-streams). I think I did fix a timeout issue there. I'll merge it soon.

lbartnik commented 7 years ago

Hi @johndharrison, could you please see if the issue has been fixed for you? It seems to be fixed in my environment.

johndharrison commented 7 years ago

Hi @lbartnik

that has fixed the issue I was having. The return from process_read where pipe != "both" is inconsistent across platforms however. Mac/osx are returning a list (for example list(stdout = "ddfsdg")) linux is returning just the character vector.

I prefer a list to be returned for all pipe settings (so that res[["stderr"]] will just return NULL if the pipe = "stdout") but either way is fine.

lbartnik commented 7 years ago

Hi @johndharrison, it's strange what you write about Mac because all platforms pass the same set of tests. I'll keep the output as it is for now but maybe you're right.

johndharrison commented 7 years ago

Hi @lbartnik yes I must not have been running the most recent branch as process_read is working as expected cross-platform now. Thanks