fixes #2136 - NPE in ExpressionOperator.printCollection

[igormukhin]

The bug is described in detail in #2136

We are looping over an array which is located in the instance field which can be set to null while lopping which is causing NPE. To fix it we are caching the value of argumentIndices by looping over an iterator of the array.

[igormukhin]

If this PR will be approved, the same thing should be done on the 3.0.x branch

[rfelcman]

I'm asking itself how this change helps for (int i = 0; i < this.argumentIndices.length; i++) { -> for (final int index : this.argumentIndices) { with NPE because in case of int[] argumentIndices = null; it throws java.lang.NullPointerException: Cannot read the array length because "<local1>" is null

[igormukhin]

@rfelcman yeah, this is very hard to spot. for (final int index : this.argumentIndices) { internally creates an iterator that reference the array. So, no matter what happens with the assignment of this.argumentIndices the iterator still references the array.

Example:

class Sample {
    private int[] fields;

    public void willWork() {
        fields = new int[] {1,2,3};
        for (int n : this.fields) {
            System.out.println(n);
            this.fields = null;
        }
    }

    public void willFail() {
        fields = new int[] {1,2,3};
        for (int i = 0; i < this.fields.length; i++) {
            int n = this.fields[i];
            System.out.println(n);
            this.fields = null;
        }
    }

    public static void main(String[] args) {
        System.out.println("Will work:");
        new Sample().willWork();

        System.out.println("Will fail:");
        new Sample().willFail();
    }
}

[rfelcman]

As others mentioned NPE under stress it looks like concurrency issue not simple NPE.

[igormukhin]

For our project the issue does not happen under stress. It just happens sometimes.

My first take on the issue was also concurrency, but we also have logs where the issue happens surely when there is only one thread.

Also the best indication for the issue is that it was introduced in 2.7.11 where the line for (final int index : this.argumentIndices) { was replaced with for (int i = 0; i < this.argumentIndices.length; i++) {

Every reporter of the problem agrees that the bug was introduced in 2.7.11.