584420 - Iteration behavior differs in 10.1 from 10.0 when deleting within nested for in loop

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Library, defect, P1)

Description

[Lars T Hansen]

Attachment: Test case

This is from https://bugs.adobe.com/jira/browse/ASL-89.

The report is that the attached test case behaves differently (and incorrectly) in 10.1:

"Actual Results:

In FP 10.1.53.64, loop quit earier then expected. (outter loop only repeat once). This behaviour is inconsistent with FP 10.0

trace output:
 
OUTER: a
[X] a

Expected Results:
 
In FP 10.0, outter loop should repeat twice.

trace output:

OUTER: b
[X] b
OUTER: a
[X] a"

Comment 1

[Lars T Hansen]

Also reported here: https://bugs.adobe.com/jira/browse/FP-4869.

Comment 2

[Steven Johnson]

Also Watson #2652554. 

Basically, a for...in loop that (1) contains
another nested for...in loop on the same object, and (2) deletes entries on
that object, used to always iterate completely, but now stops prematurely.

   var d = { a:"a", b:"b", c:"c", d:"d"};
   for (var o in d)
   {
      for (var o2 in d) { /*whatever, doesn't matter*/ }
      trace("deleting ",o);
      delete d[o];
   }

formerly would always trace out all 4 keys, but now stops prematurely
(typically after two keys).

Note that the Watson/Jira bug was reported for Dictionary, but it actually occurs
for any object.

The issue here is that the code that deals with the "public" index vs. "real"
index for enumeration doesn't take into account the possibility that elements
can be deleted during enumeration.

Right now this is marked for fixing in Salt, but a truly robust fix is going to be tricky... I can think of ways to deal with the delete-an-item-during-iteration issue, but there will still be ways to subvert the process (eg, delete and insert items in arbitrary places during iteration)... a proper fix may require overhauling our member iteration in general.

Comment 3

[Lars T Hansen]

I'm in favor of fixing the for-in loop in general, though for backward compatibility reasons we may have to fix the present issue in isolation.

Comment 4

[Steven Johnson]

(In reply to comment #3)
> I'm in favor of fixing the for-in loop in general, though for backward
> compatibility reasons we may have to fix the present issue in isolation.

Agreed on both counts. Unfortunately, the proper fix probably needs to be versioned, so fixing this will probably require an ugly band-aid...

Comment 5

[Wenwei Liao]

(In reply to comment #2)
> Also Watson #2652554. 
> 
> Basically, a for...in loop that (1) contains
> another nested for...in loop on the same object, and (2) deletes entries on
> that object, used to always iterate completely, but now stops prematurely.
> 
>    var d = { a:"a", b:"b", c:"c", d:"d"};
>    for (var o in d)
>    {
>       for (var o2 in d) { /*whatever, doesn't matter*/ }
>       trace("deleting ",o);
>       delete d[o];
>    }
> 
> formerly would always trace out all 4 keys, but now stops prematurely
> (typically after two keys).
> 
> Note that the Watson/Jira bug was reported for Dictionary, but it actually
> occurs
> for any object.
> 
> The issue here is that the code that deals with the "public" index vs. "real"
> index for enumeration doesn't take into account the possibility that elements
> can be deleted during enumeration.
> 
> Right now this is marked for fixing in Salt, but a truly robust fix is going to
> be tricky... I can think of ways to deal with the
> delete-an-item-during-iteration issue, but there will still be ways to subvert
> the process (eg, delete and insert items in arbitrary places during
> iteration)... a proper fix may require overhauling our member iteration in
> general.

Something strange also happened when iterating array elements between FP10.0 and FP10.1 based on the test case you provided:

    var a:Array = ["111", "222", "333", "444", "555"];
    for (var k1:* in a) {
        for (var k2:* in a) {}
        trace("deleting ", k1);
        delete a[k1];
    }

FP10.0 reports: 

    deleting  0
    deleting  1
    deleting  2
    deleting  3
    deleting  4

FP10.1 reports:

    deleting  0
    deleting  2
    deleting  4

This is very similar to your result of testing iterating Object/Dictionary objects. But strange things happened when I removed the statement "for (var k2:* in a) {}" which makes the test case looks like:

    var a:Array = ["111", "222", "333", "444", "555"];
    for (var k1:* in a) {
        trace("deleting ", k1);
        delete a[k1];
    }

FP10.0 gave the exact same result as the previous test case, but FP10.1 reports:

    deleting  0
    deleting  2
    deleting  3
    deleting  4

I think there must be something happened in the iteration implementation between the two FP versions. Although it is surely not a good practice that removing an element from a object/array while iterating the same one. But the inconsistent behavior between FP10.0 and FP10.1 (even between Object and Array both in FP10.1) makes me a little confused.

Comment 6

[Trevor Baker]

regarding comment #4, Salt will be versioned as SWF11

Comment 7

[Steven Johnson]

(In reply to comment #6)
> regarding comment #4, Salt will be versioned as SWF11

Are you sure about that? Seems highly unlikely, as the BugCompatibility code for SWF11 isn't landing in Salt...

Comment 8

[Dan Smith]

The decision was made to not version Salt as SWF11.

Comment 9

[Dan Smith]

QRB: review WE bug where Rick and Steven recommend classification as WONTFIX.

Comment 10

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 11

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.