Skip to content

Commit f4489d5

Browse files
HenrikBengtssonHenrikBengtsson
committed
Remove special handling of globals from Multicore- and UniprocessFuture
1 parent 5c032a6 commit f4489d5

File tree

6 files changed

+52
-27
lines changed

6 files changed

+52
-27
lines changed

‎R/MulticoreFuture-class.R‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,18 +48,11 @@ run.MulticoreFuture <- function(future, ...) {
4848

4949
mcparallel <- importParallel("mcparallel")
5050

51-
expr <- getExpression(future, globals = list(), cleanup = FALSE)
51+
expr <- getExpression(future)
5252
envir <- future$envir
53-
envir <- new.env(parent = envir)
5453

5554
t_start <- Sys.time()
5655

57-
## Assign globals
58-
globals <- future$globals
59-
if (length(globals) > 0L) {
60-
envir <- assign_globals(envir, globals = globals)
61-
}
62-
6356
## Get a free worker
6457
reg <- sprintf("multicore-%s", session_uuid())
6558
requestCore(

‎R/UniprocessFuture-class.R‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,8 @@ run.UniprocessFuture <- function(future, ...) {
3333
## also the one that evaluates/resolves/queries it.
3434
assertOwner(future)
3535

36-
expr <- getExpression(future, globals = list())
36+
expr <- getExpression(future)
3737
envir <- future$envir
38-
envir <- new.env(parent = envir)
39-
40-
## Assign globals to separate "globals" enclosure environment?
41-
globals <- future$globals
42-
if (length(globals) > 0) {
43-
envir <- assign_globals(envir, globals = globals, debug = debug)
44-
}
4538

4639
## Run future
4740
future$state <- 'running'

‎R/expressions.R‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -445,15 +445,32 @@ evalFuture <- function(
445445
...future.globalenv.names <- c(names(.GlobalEnv), "...future.value", "...future.globalenv.names", ".Random.seed")
446446
}
447447

448-
## Attach globals
448+
## Attach globals to the global environment
449+
## Undo changes on exit
449450
if (length(globals) > 0) {
450-
base_attach <- base::attach ## To please R CMD check
451-
base_attach(globals, pos = 2L, name = "future:globals", warn.conflicts = FALSE)
452-
if (cleanup) {
453-
on.exit({
454-
detach(name = "future:globals")
455-
}, add = TRUE)
451+
## Preserve globals
452+
genvOld <- new.env(parent = emptyenv())
453+
genv <- globalenv()
454+
for (name in names(globals)) {
455+
if (exists(name, envir = genv, inherits = FALSE)) {
456+
value <- get(name, envir = genv, inherits = FALSE)
457+
assign(name, value = value, envir = genvOld, inherits = FALSE)
458+
}
456459
}
460+
on.exit({
461+
## Remove globals from the global environment
462+
rm(list = names(globals), envir = genv, inherits = FALSE)
463+
## Restore overwritten objects in the global environment
464+
for (name in names(genvOld)) {
465+
if (exists(name, envir = genvOld, inherits = FALSE)) {
466+
value <- get(name, envir = genvOld, inherits = FALSE)
467+
assign(name, value = value, envir = genv, inherits = FALSE)
468+
}
469+
}
470+
rm(list = "genvOld")
471+
}, add = TRUE)
472+
473+
assign_globals(globalenv(), globals = globals)
457474
}
458475

459476
conditionClassesExclude <- attr(conditionClasses, "exclude", exact = TRUE)

‎tests/futureCall.R‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,13 +141,16 @@ for (cores in 1:availCores) {
141141
utils::str(list(strategy = strategy, globals = globals, lazy = lazy, v4 = v4))
142142

143143
if (isTRUE(as.logical(Sys.getenv("R_CHECK_IDEAL")))) {
144+
message("R_CHECK_IDEAL=TRUE")
144145
if (globals) {
145146
stopifnot(identical(v4, truth))
146147
} else {
147148
stopifnot(inherits(v4, "error"))
148149
}
149150
} else if (isTRUE(getOption("future.globals.keepWhere", FALSE))) {
151+
message("future.globals.keepWhere=TRUE")
150152
if (isTRUE(getOption("future.globals.globalsOf.locals", TRUE))) {
153+
message("future.globals.globalsOf.locals=TRUE")
151154
if (globals) {
152155
stopifnot(identical(v4, truth))
153156
} else if (lazy) {
@@ -156,24 +159,28 @@ for (cores in 1:availCores) {
156159
stopifnot(identical(v4, truth))
157160
}
158161
} else {
162+
message("future.globals.globalsOf.locals=FALSE")
159163
if (lazy) {
160164
stopifnot(inherits(v4, "error"))
161165
} else {
162166
stopifnot(identical(v4, truth))
163167
}
164168
}
165169
} else {
170+
message("future.globals.keepWhere=FALSE")
166171
if (isTRUE(getOption("future.globals.globalsOf.locals", TRUE))) {
172+
message("future.globals.globalsOf.locals=TRUE")
167173
if (globals) {
168174
stopifnot(identical(v4, truth))
169175
} else if (lazy) {
170176
stopifnot(inherits(v4, "error"))
171177
} else if (strategy %in% c("sequential", "multicore")) {
172-
stopifnot(inherits(v4, "error"))
173-
} else {
174178
stopifnot(identical(v4, truth))
179+
} else {
180+
stopifnot(inherits(v4, "error"))
175181
}
176182
} else {
183+
message("future.globals.globalsOf.locals=FALSE")
177184
if (strategy %in% c("sequential", "multicore")) {
178185
stopifnot(inherits(v4, "error"))
179186
} else if (lazy) {

‎tests/globals,S4methods.R‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,23 @@ for (strategy in supportedStrategies()) {
3232
v <- tryCatch(value(f), error = identity)
3333
print(v)
3434
if (isTRUE(as.logical(Sys.getenv("R_CHECK_IDEAL")))) {
35+
message("R_CHECK_IDEAL=TRUE")
3536
if (getOption("future.globals.keepWhere", TRUE)) {
37+
message("future.globals.keepWhere=TRUE")
3638
stopifnot(identical(v, truth))
3739
} else {
40+
message("future.globals.keepWhere=FALSE")
3841
stopifnot(inherits(v, "error"))
3942
}
4043
} else if (isTRUE(getOption("future.globals.keepWhere", FALSE))) {
44+
message("future.globals.keepWhere=TRUE")
4145
stopifnot(identical(v, truth))
4246
} else {
47+
message("future.globals.keepWhere=FALSE")
4348
if (strategy %in% c("sequential", "multicore")) {
4449
stopifnot(inherits(v, "error"))
4550
} else {
46-
stopifnot(identical(v, truth))
51+
stopifnot(inherits(v, "error"))
4752
}
4853
}
4954
my_fcn <- org_my_fcn

‎tests/globals,locals.R‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,16 +83,20 @@ for (strategy in supportedStrategies()) {
8383
if (isTRUE(as.logical(Sys.getenv("R_CHECK_IDEAL")))) {
8484
stopifnot(identical(v, truth))
8585
} else if (isTRUE(getOption("future.globals.keepWhere", FALSE))) {
86+
message("future.globals.keepWhere=TRUE")
8687
if (isTRUE(getOption("future.globals.globalsOf.locals", TRUE))) {
88+
message("future.globals.globalsOf.locals=TRUE")
8789
if (strategy %in% c("sequential", "multicore")) {
8890
stopifnot(inherits(res, "error"))
8991
} else {
9092
stopifnot(identical(v, truth))
9193
}
9294
} else {
95+
message("future.globals.globalsOf.locals=FALSE")
9396
stopifnot(identical(v, truth))
9497
}
9598
} else {
99+
message("future.globals.keepWhere=FALSE")
96100
stopifnot(identical(v, truth))
97101
}
98102
})
@@ -129,19 +133,25 @@ for (strategy in supportedStrategies()) {
129133
if (isTRUE(as.logical(Sys.getenv("R_CHECK_IDEAL")))) {
130134
stopifnot(identical(v, truth))
131135
} else if (isTRUE(getOption("future.globals.keepWhere", FALSE))) {
136+
message("future.globals.keepWhere=TRUE")
132137
if (isTRUE(getOption("future.globals.globalsOf.locals", TRUE))) {
138+
message("future.globals.globalsOf.locals=TRUE")
133139
stopifnot(identical(v, truth))
134140
} else {
141+
message("future.globals.globalsOf.locals=FALSE")
135142
stopifnot(identical(v, truth))
136143
}
137144
} else {
145+
message("future.globals.keepWhere=FALSE")
138146
if (isTRUE(getOption("future.globals.globalsOf.locals", TRUE))) {
147+
message("future.globals.globalsOf.locals=TRUE")
139148
if (strategy %in% c("sequential", "multicore")) {
140149
stopifnot(identical(v, 4)) ## <= SERIOUS BUG!
141150
} else {
142-
stopifnot(identical(v, truth))
151+
stopifnot(identical(v, 4)) ## <= SERIOUS BUG!
143152
}
144153
} else {
154+
message("future.globals.globalsOf.locals=FALSE")
145155
if (strategy %in% c("sequential", "multicore")) {
146156
stopifnot(inherits(v, "error"))
147157
} else {

0 commit comments

Comments
 (0)