tidyverse / magrittr

Improve the readability of R code with the pipe
https://magrittr.tidyverse.org
Other
957 stars 157 forks source link

Stack overflow from huge pipe #196

Closed MichaelChirico closed 5 years ago

MichaelChirico commented 5 years ago

Was playing around with scaling piping and crashed R:

library(magrittr)
build_pipe = function(n) {
  paste0('1 %>% add(', paste(2:n, collapse = ') %>% add('), ')')
}
eval(parse(text = build_pipe(10000L)))
# [1] 50005000 # OK
eval(parse(text = build_pipe(100000L)))

Error: protect(): protection stack overflow Segmentation fault: 11

parseing works fine, it's the evaluation that crashes:

parse(text = build_pipe(100000L))

It's also funny that magrittr skips nested recursion limits imposed by options('expressions'):

old = options(expressions = 5000L)
eval(parse(text = paste(1:10000, collapse = ' + ')))

Error: evaluation nested too deeply: infinite recursion / options(expressions=)?

options(expressions = 500000L)
eval(parse(text = paste(1:10000, collapse = ' + ')))
# [1] 50005000 # OK again
eval(parse(text = paste(1:100000, collapse = ' + ')))

Error: C stack usage 7969328 is too close to the limit

# resetting
options(old)

That is, the same stack overflow is caught and crashed gracefully without %>%

brodieG commented 5 years ago

Fyi, the recursion aspect is likely affected by the changes between the CRAN version and the github version: https://github.com/tidyverse/magrittr/commit/5fea9abf7c63dcb59f08cd7704a2dad11f97b858#diff-64d2ab9799f460c863110d704757bd66

brodieG commented 5 years ago

FWIW, the segfault probably has nothing to do with magrittr:

xx <- parse(text = build_pipe(100000L))
xx[[1]]
## Error: segfault from C stack overflow

Having a pair list that deeply nested seems to throw a wrench into some part of the internals.

MichaelChirico commented 5 years ago

I see. Will close here in that case, maybe I'll raise to r-devel.