Skip to content

Commit 3645d33

Browse files
with(<plan>, ...): avoid cleaning up current plan
1 parent 4a6dd9e commit 3645d33

File tree

5 files changed

+60
-21
lines changed

5 files changed

+60
-21
lines changed

R/utils_api-plan.R

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -72,19 +72,48 @@
7272
}) ## plan_default_stack()
7373

7474

75-
plan_cleanup <- function(evaluator) {
76-
cleanup <- attr(evaluator, "cleanup", exact = TRUE)
77-
if (!is.null(cleanup)) {
78-
if (is.function(cleanup)) {
79-
cleanup()
80-
} else {
81-
stop(FutureError(sprintf("Unknown type of 'cleanup' attribute on current future strategy: %s", commaq(class(cleanup)))))
75+
plan_cleanup <- function(evaluator, cleanup = NA) {
76+
## Nothing to do?
77+
if (identical(cleanup, FALSE)) return()
78+
79+
cleanup_fcn <- attr(evaluator, "cleanup", exact = TRUE)
80+
81+
## Nothing to do?
82+
if (is.null(cleanup_fcn) &&
83+
!isTRUE(getOption("future.plan.cleanup.legacy"))) {
84+
return()
85+
}
86+
87+
## Skip clean up for other reasons?
88+
if (is.na(cleanup)) {
89+
## Skip because this was called via with(plan(...), ...)?
90+
calls <- sys.calls()
91+
ncalls <- length(calls)
92+
if (ncalls > 3L) {
93+
for (ii in (ncalls-3L):1) {
94+
call <- calls[[ii]]
95+
fcn <- call[[1]]
96+
if (is.symbol(fcn) && fcn == as.symbol("with")) {
97+
return()
98+
} else if (is.call(fcn) &&
99+
is.symbol(fcn[[1]]) && fcn[[1]] == as.symbol("::") &&
100+
is.symbol(fcn[[2]]) && fcn[[2]] == as.symbol("base") &&
101+
is.symbol(fcn[[3]]) && fcn[[3]] == as.symbol("with")) {
102+
return()
103+
}
104+
}
82105
}
83-
} else {
106+
}
107+
108+
## Clean up
109+
if (is.function(cleanup_fcn)) {
110+
message("- cleanup()")
111+
cleanup_fcn()
112+
} else if (is.null(cleanup_fcn)) {
84113
## Backward compatibility for future (<= 1.33.2)
85-
if (isTRUE(getOption("future.plan.cleanup.legacy"))) {
86-
ClusterRegistry(action = "stop")
87-
}
114+
ClusterRegistry(action = "stop")
115+
} else {
116+
stop(FutureError(sprintf("Unknown type of 'cleanup' attribute on current future strategy: %s", commaq(class(cleanup_fcn)))))
88117
}
89118
} ## plan_cleanup()
90119

@@ -270,7 +299,7 @@ plan <- local({
270299
## Stack of type of futures to use
271300
stack <- NULL
272301

273-
plan_set <- function(newStack, skip = TRUE, cleanup = TRUE, init = TRUE) {
302+
plan_set <- function(newStack, skip = TRUE, cleanup = NA, init = TRUE) {
274303
stop_if_not(!is.null(newStack), is.list(newStack), length(newStack) >= 1L)
275304

276305
oldStack <- stack
@@ -298,7 +327,7 @@ plan <- local({
298327
warn_about_multicore(newStack)
299328

300329
## Stop/cleanup any previously registered backends?
301-
if (cleanup) plan_cleanup(stack[[1L]])
330+
plan_cleanup(stack[[1L]], cleanup = cleanup)
302331

303332
stack <<- newStack
304333

@@ -324,7 +353,7 @@ plan <- local({
324353

325354
## Main function
326355
function(strategy = NULL, ..., substitute = TRUE, .skip = FALSE, .call = TRUE,
327-
.cleanup = TRUE, .init = TRUE) {
356+
.cleanup = NA, .init = TRUE) {
328357
if (substitute) strategy <- substitute(strategy)
329358
if (is.logical(.skip)) stop_if_not(length(.skip) == 1L, !is.na(.skip))
330359
if (is.logical(.call)) stop_if_not(length(.call) == 1L, !is.na(.call))
@@ -352,7 +381,7 @@ plan <- local({
352381
return(stack)
353382
} else if (identical(strategy, "reset")) {
354383
## Stop/cleanup any previously registered backends?
355-
if (.cleanup) plan_cleanup(stack[[1]])
384+
plan_cleanup(stack[[1]], cleanup = .cleanup)
356385
## Reset stack of future strategies?
357386
stack <<- plan_default_stack()
358387
return(stack)

R/utils_api-with.R

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,20 @@
1111
#' @example incl/with.R
1212
#'
1313
#' @export
14-
with.FutureStrategyList <- function(data, expr, ...) {
15-
on.exit(plan(data))
16-
plan("next")
14+
with.FutureStrategyList <- function(data, expr, ...) {
15+
## At this point, 'data' has already been resolved by
16+
## R's dispatching mechanism. At this point, it is
17+
## too late to override with .cleanup = FALSE.
18+
old_plan <- data
19+
on.exit({
20+
## Always cleanup the temporarily used backend, iff there is a
21+
## cleanup() function declared.
22+
## Note, we do it here for now, to avoid legacy cleanup in plan()
23+
cleanup <- attr(plan("next"), "cleanup", exact = TRUE)
24+
if (is.function(cleanup)) cleanup()
25+
26+
plan(old_plan, .cleanup = FALSE)
27+
})
28+
1729
expr
1830
}

incl/with.R

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,3 @@ with(plan(multisession, workers = 2), {
44
w_pid <- value(f)
55
c(main = Sys.getpid(), worker = w_pid)
66
})
7-

man/plan.Rd

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

man/with.FutureStrategyList.Rd

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)