Closed Bug 1334799 Opened 4 years ago Closed 4 years ago

Iterator is not closed when breaking from finally

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox-esr45 --- unaffected
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: arai, Assigned: arai)

References

Details

Attachments

(1 file)

similar to bug 1332881.

https://dxr.mozilla.org/mozilla-central/rev/045d8fe30f546ab08466c9586ce298e6459c2069/js/src/frontend/BytecodeEmitter.cpp#2405-2427
flushPops should be done *before* handling target ForOfLoopControl :/

code:

var called = false;
var obj = {
  [Symbol.iterator]() {
    return {
      next() {
        return { value: 10, done: false };
      },
      return() {
        called = true;
        return {};
      }
    };
  }
};

for (var x of obj) {
  try {
  } finally {
    break;
  }
}
assertEq(called, true);


this time it doesn't cause crash, but just doesn't close iterator.
I should've checked that iterator is properly closed in bug 1332881's testcase.
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Attachment #8831438 - Flags: review?(shu)
Attachment #8831438 - Flags: review?(shu) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/34aa0752cd03e7c062d67ccbff83712482ce1447
Bug 1334799 - Handle stack value in correct order when leaving for-of loop from finally block. r=shu
https://hg.mozilla.org/mozilla-central/rev/34aa0752cd03
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Blocks: 1147371
Comment on attachment 8831438 [details] [diff] [review]
Handle stack value in correct order when leaving for-of loop from finally block.

Approval Request Comment
> [Feature/Bug causing the regression]
the code is added in bug 1147371,
and overlooked part in bug 1332881.

> [User impact if declined]
Wrong JavaScript behavior

> [Is this code covered by automated tests?]
Yes

> [Has the fix been verified in Nightly?]
Yes

> [Needs manual test from QE? If yes, steps to reproduce]
No

> [List of other uplifts needed for the feature/fix]
None

> [Is the change risky?]
Less risky

> [Why is the change risky/not risky?]
this changes the target stack offset of operations, like bug 1332881.
now testcase confirms it's really correct.

> [String changes made/needed]
None
Attachment #8831438 - Flags: approval-mozilla-aurora?
Comment on attachment 8831438 [details] [diff] [review]
Handle stack value in correct order when leaving for-of loop from finally block.

Fixes a JS behavior, Aurora53+
Attachment #8831438 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.