Avoid java.io.NotSerializableException: java.util.ArrayList$Itr

[nevskrem]

Avoid java.io.NotSerializableException: java.util.ArrayList$Itr:
in field com.cloudbees.groovy.cps.impl.ForInLoopBlock$ContinuationImpl.itr
in object com.cloudbees.groovy.cps.impl.ForInLoopBlock$ContinuationImpl@7f282ca0
in field com.cloudbees.groovy.cps.impl.ContinuationPtr$ContinuationImpl.target
in object com.cloudbees.groovy.cps.impl.ContinuationPtr$ContinuationImpl@57319f0f
in field com.cloudbees.groovy.cps.impl.LoopBlockScopeEnv.continue_
...

[dwnusbaum]

Thanks for the PR! Making itr transient will fix the NotSerializableException, but I think it will also cause the semantics to be incorrect after the script is resumed, since the state of the iteration will not be saved.

I would've thought that IteratorHack in workflow-cps, which uses Groovy category methods to intercept creation of Iterators and similar types would intercept this and make it serializable as well, but maybe that's not the case? If that is the problem, I think the fix would be to see if we can modify line 52 so that the creation of the Iterator is intercepted by IteratorHack, if possible. I am surprised we haven't seen this before, but maybe it's just because this syntax is not used very often?

It would be great to add a regression test for this issue at the same time.

[nevskrem]

@dwnusbaum honestly speaking it was pretty clear upfront that state persistence will be gone after making it transient but I wanted to get the discussion initiated quickly - thanks for picking it up.

Unfortunately I am not able to reproduce the issue with a test, it occurs sporadically in one of our pipelines and we are not yet sure about the exact dependencies that let it evolve.

So where would the IteratorHack get hooked on the collections in use? Most likely the SkriptByteCodeAdapter is the root cause of seeing the original implementation being returned instead of the hacked version.

Could you share your view on that?

[nevskrem]

Found our issue which is basically related to the TODO still documented in IteratorHack about Java 5 style for loops.

[dwnusbaum]

@nevskrem If you have a minimal reproducer, we could still add it as a test here for the future to make it easier to understand the conditions in which this is a problem.

[nevskrem]

@dwnusbaum thanks for offering but at this point in time I can actually not invest, sorry. We use pipeline unit testing so I do not have a single test at hand. Besides that it is strong related to the use of Java-5 style for loops (https://github.com/SAP/jenkins-library/pull/1262/files/7119f16e49d13e4a4210e6d2f75cf7b816275b8a#diff-9bc4ba5f12fa615c35977ecfd46bd1db) in combination with waiting theats. So in our scenario as part of the body being executed we perform HTTP requests that sometimes wait a bit longer for the response and then after some minutes the executor starts switching tasks and with that the problem starts. Upon the attempt to serialize the context it runs into some issue with this type of loop.