Closures in Groovy not capturing outside variables
Asked Answered
L

1

6

In the context of Jenkins pipelines, I have some Groovy code that's enumerating a list, creating closures, and then using that value in the closure as a key to lookup another value in a map. This appears to be rife with some sort of anomaly or race condition almost every time.

This is a simplification of the code:

def tasks = [:]
for (platformName in platforms) {
  // ...

  tasks[platformName] = {
    def componentUploadPath = componentUploadPaths[platformName]

    echo "Uploading for platform [${platformName}] to [${componentUploadPath}]."

    // ...
}

tasks.failFast = true
parallel(tasks)

platforms has two values. I will usually see two iterations and two tasks registered and the keys in tasks will be correct, but the echo statement inside the closure indicates that we're just running one of the platforms twice:

14:20:02 [platform2] Uploading for platform [platform1] to [some_path/platform1].
14:20:02 [platform1] Uploading for platform [platform1] to [some_path/platform1].

It's ridiculous.

What do I need to add or do differently?

Looby answered 28/5, 2019 at 18:43 Comment(1)
"almost every time"? So you saw it working? – Parke
K
9

It's the same issue as you'd see in Javascript.

When you generate the closures in a for loop, they are bound to a variable, not the value of the variable.

When the loop exits, and the closures are run, they will all be using the same value...that is -- the last value in the for loop before it exited

For example, you'd expect the following to print 1 2 3 4, but it doesn't

def closures = []

for (i in 1..4) {
    closures << { -> println i }
}

closures.each { it() }

It prints 4 4 4 4

To fix this, you need to do one of two things... First, you could capture the value in a locally scoped variable, then close over this variable:

for (i in 1..4) {
    def n = i
    closures << { -> println n }
}

The second thing you could do is use groovy's each or collect as each time they are called, the variable is a different instance, so it works again:

(1..4).each { i ->
    closures << { -> println i }
}

For your case, you can loop over platforms and collect into a map at the same time by using collectEntries:

def tasks = platforms.collectEntries { platformName ->
  [
     platformName,
     { ->
        def componentUploadPath = componentUploadPaths[platformName]
        echo "Uploading for platform [${platformName}] to [${componentUploadPath}]."
     }
  ]
}

Hope this helps!

Kellby answered 28/5, 2019 at 18:45 Comment(5)
What about just using $it? It seems to me that, in this case, Groovy will automatically assign the key from the map as $it in each closure, so it's just a matter of setting def platformName = $it. – Looby
With the for loop? There is no it – Kellby
No worries πŸ˜€πŸ‘ glad I could help – Kellby
Sorry, just noticed my each example was overly complex. Fixed now 😎 – Kellby
closures*.call() for the golf then ;) – Parke

© 2022 - 2024 β€” McMap. All rights reserved.