Bugzilla – Bug 324708
[Flow Analysis] goto does not call the finally block to close the iterator when leaving a foreach loop
Last modified: 2008-04-05 20:19:46 UTC
---- Reported by jeff@thefirst.org 2007-07-07 13:21:37 MST ---- Please fill in this template when reporting a bug, unless you know what you are doing. Description of Problem: Usually, in an iterator, we can put yield inside a try/finally block and when control leaves the foreach loop for the iterator (because of "break" or an exception) it will call the iterator's finally block. But if we leave the foreach loop using goto, it doesn't do this. But we use "goto" to transfer control out of one or more nested loops, and it should behave the same as "break". Steps to reproduce the problem: 1. Compile and run this code: using System.Collections.Generic; class TestGoto { static int x = 0; static void Main(string[] args) { foreach (bool b in test()) { } } static IEnumerable<bool> setX() { x = 1; try { yield return true; } finally { x = 0; } } static IEnumerable<bool> test() { foreach (bool b in setX()) { yield return true; // Change "goto label" to "break" to show the correct result. goto label; } label: if (x == 0) System.Console.WriteLine("Success: finally was called"); else System.Console.WriteLine("Error: finally was not called"); } } Actual Results: prints "Error: finally was not called" because the setX() iterator is not closed when leaving the foreach loop. Expected Results: If you change "goto label" to "break", then the iterator is closed properly and it prints "Success: finally was called". But a "goto" should act like "break" and also close the iterator. How often does this happen? Every time Additional Information: ---- Additional Comments From jeff@thefirst.org 2007-07-13 00:11:08 MST ---- Microsoft has confirmed the same bug in their C# compiler and will fix it in the next release: https://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx ?FeedbackID=286401 Unknown bug field "cf_op_sys_details" encountered while moving bug <cf_op_sys_details>XP</cf_op_sys_details> Unknown bug field "cf_version_details" encountered while moving bug <cf_version_details>1.2.4</cf_version_details> Unknown operating system unknown. Setting to default OS "Other".
This same bug is confirmed in Microsoft C# 2008. Has anyone reviewed this bug for Mono?
That should have said "This same bug is confirmed fixed in Microsoft C# 2008" Has anyone reviewed this bug for Mono?
The issue is that we emit hidden dispose section after closing foreach `}'. But standard goto statement does not about this section. Fixing this simple case may be easy but it's quite tricky to fix global goto jump to any instruction. We should probably emit some helper method which gets calls when the goto leaves the scope of any disposable iterator.
Reassigning to Hari.
The 'goto' statement does know about it, and uses the 'leave' instruction. However, since it's inside an iterator, the "finally" clauses are emitted inline instead of inside a finally handler, and thus the 'leave' has no effect. One solution is to keep the unwind-protect areas as-is inside iterators, and branch around any 'finally' clauses with a bool that's set by a 'yield return'. ExceptionStatement::DoEmitFinally(): Label endfinally BeginFinallyHandler() if (InIterator) emit (ldloc, "unwind_protect") emit (brfalse, endfinally) EmitFinally () MarkLabel (endfinally) EndExceptionBlock() Iterator::MarkYield(): emit (ld.i4.0) emit (stloc, "unwind_protect") ...emit loading PC and jump to ret... Iterator::EmitMoveNext(): change outer try/finally to a try/fault handler Let me see if this works out.
That last change isn't necessary: MoveNext is already emitted as a try/fault block.
Unfortunately that isn't enough, since we can't jump into a try statement. I'm working on a series of patches to clean up this area.
After a long series of cleanups, this should be fixed in SVN r99929.