tracker issue : CF-4010308

select a category, or use search below
(searches all categories and all time range)
Title:

for-in bug

| View in Tracker

Status/Resolution/Reason: Closed/Fixed/

Reporter/Name(from Bugbase): Lewis Stewart / Lewis Stewart (Lewis Stewart)

Created: 06/20/2015

Components: Language

Versions: 10.0

Failure Type: Performance Issue

Found In Build/Fixed In Build: Final /

Priority/Frequency: Major / All users will encounter

Locale/System: English / Linux All

Vote Count: 1

Listed in the version 2016.0.0.297996 Issues Fixed doc
Verification notes: verified_fixed on August 24, 2019 using build 2016.0.01.298513
I believe there is a serious bug with the for-in construct when used in a particular fashion. In particular, if the "in" expression is a function call, it appears that the function is invoked multiple times (to be precise, it always appears to be 5 times).

Clearly there are many situations where this could have serious consequences, not least if the function does any serious work or has side effects.

Key points:

Tested on Coldfusion 10 update 16
Using a for-in construct where the "in" expression invokes a function results in that function being invoked multiple times.
It doesn't seem to matter what the function returns (e.g. struct, array, potentially other iterables)

Workaround:

Assign the result of the function call to a local variable outside the for-in construct

Severity:

This a serious issue as,

It is easy to cause using a common idiom
It is completed unexpected
The fact that it is happening is essentially invisible to the developer
The results are potentially very serious

Speculation:

I speculate that the code is pre-processed into a construct that does (in clause).size() (in clause).iterator(), etc

Test Case:

See below for some code which tests this:

component displayname="ForInTest" {

  ForInTest function init() {
    variables.count = 0;
    return this;
  }

  void function test1() {
    variables.count = 0;

    for( var item in getArray() ) {
      // do something interesting
    }

    if( variables.count==1 ) {
      writeoutput( "getArray() was called once." );
    }
    else {
      writeoutput( "getArray() was called multiple times! (" & variables.count & ")" );
    }
  }

  void function test2() {
    variables.count = 0;

    var items = getArray();
    for( var item in items ) {
      // do something interesting
    }

    if( variables.count==1 ) {
      writeoutput( "getArray() was called once." );
    }
    else {
      writeoutput( "getArray() was called multiple times! (" & variables.count & ")" );
    }
  }

  private array function getArray() {
    variables.count++;
    var data = [1,2,3];
    return data;
  }
}

var test = new ForInTest();
test.test1();
test.test2();

Output:
getArray() was called multiple times! (5)
getArray() was called once.

----------------------------- Additional Watson Details -----------------------------

Watson Bug ID:	4010308

External Customer Info:
External Company:  
External Customer Name: Lewis
External Customer Email:  
External Test Config: My Hardware and Environment details:

Attachments:

Comments:

Do you think you could put that code in a gist or something so its indentation etc is preserved? It makes for hard reading.
Comment by External U.
7187 | June 20, 2015 04:27:23 PM GMT
The repro can be greatly simplified to just this: https://github.com/adamcameron/cfml/blob/master/vendor/ColdFusion/bugs/forin.cfm function getChars(){ writeOutput("getChars() called (should see this ONCE)<br>"); return ["a", "b", "c"]; } for (c in getChars()){ writeOutput("#c#<br>"); } Actual: getChars() called (should see this ONCE) getChars() called (should see this ONCE) getChars() called (should see this ONCE) getChars() called (should see this ONCE) getChars() called (should see this ONCE) a b c Expected: getChars() called (should see this ONCE) a b c Lucee works properly.
Comment by External U.
7188 | June 20, 2015 04:44:26 PM GMT
Originally posted with formatting here: https://forums.adobe.com/thread/1873440
Comment by External U.
7189 | June 21, 2015 03:21:42 AM GMT
Interestingly... in my example it always calls the function five times, irrespective of the length of the array. And note they all take place before the iteration takes place. Same applies with structs, although it's called six times then. And queries (forget how many times it's called then).
Comment by External U.
7190 | June 21, 2015 03:26:37 AM GMT
If you look at the generated code you can see what's happening and it's as expected: (invoke fn) instanceof String (invoke fn) != null (invoke fn).getClass().isArray() (invoke fn) instanceof List (invoke fn) instanceof QueryTable .... etc See here: https://gist.github.com/anonymous/ccf2ba8452a1a1899495
Comment by External U.
7191 | June 21, 2015 04:19:19 AM GMT
For-in loop has been little restructured to avoid multiple calls. Now only a single call will be made.
Comment by Milan C.
7192 | September 26, 2015 05:59:03 AM GMT
+1 ......................
Vote by External U.
7193 | September 29, 2015 12:43:10 AM GMT
Hi Adobe, I've verified this is fixed in CF2016 Update 1 (build 2016.0.01.298513). Thanks!, -Aaron
Comment by Aaron N.
31172 | August 24, 2019 06:56:14 AM GMT