Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

VB.NET Iterator Function Loses Local Variables

After a couple showstoppers delayed migration to the .NET 4.6 runtime I was finally comfortable with moving to the C#6/VB14 compilers until I came across a critical issue with iterator functions in VB.NET throwing away local variables.

The following code example will throw a null reference exception on the commented line when compiled in release mode (optimized) in Visual Studio 2015 / msbuild.

Module Module1

    Sub Main()
        For Each o As Integer In GetAllStuff()
            Console.WriteLine(o.ToString())
        Next

        Console.ReadKey()

    End Sub

    Private Iterator Function GetAllStuff() As IEnumerable(Of Integer)
        Dim map As Dictionary(Of String, String) = New Dictionary(Of String, String)
        Dim tasks As New List(Of Integer)
        tasks.Add(1)

        For Each task As Integer In tasks
            Yield task
        Next

        'The value of map becomes null here under the new VB14 compiler in Release on .NET 4.6'
        For Each s As String In map.Values
            Yield 100
        Next
    End Function

End Module

So, that's pretty frightening.

Notably the C# equivalent of this code executes without issue. More importantly, this works (and has worked) under previous versions of the VB compiler. Comparing the MSIL between the state machine created by the two different compilers, the new compiler appears to be using .locals for local variable storage nearly exclusively while the old compiler was using mutable fields on the state machine for saving off local values.

Am I missing something? I was not able to find any documentation of a breaking change with iterators in VB (nor can I imagine that would be the case), but also have not found anyone else having hit this issue.

This particular example can be worked around by moving the construction of map to after the first foreach loop, however my concern is that I do not have any sense of the true flavor of this issue. I'm not interested in modifying code to "just make it work." Where else in our extensive codebase might I run into an issue a same vein? I have submitted the issue on Connect but that often feels like a black hole.

UPDATE

Someone just reported the same issue with the async state machine on the Roslyn GitHub page: https://github.com/dotnet/roslyn/issues/9001

Hopefully this starts to get a little attention.

like image 673
roken Avatar asked Feb 23 '16 03:02

roken


1 Answers

First of all, thanks for bringing some attention to the issue I raised with the Roslyn team.

I have pulled the latest Roslyn source from https://github.com/dotnet/roslyn (master branch), and added an extra unit test to the BasicCompilerEmitTest project which looks like this:

Imports Microsoft.CodeAnalysis.VisualBasic.UnitTests

Public Class KirillsTests
  Inherits BasicTestBase

  <Fact>
  Public Sub IteratorVariableCaptureTest()
    Dim source =
<compilation name="Iterators">
  <file name="a.vb">
Imports System
Imports System.Collections.Generic

Module Module1

    Sub Main()
        For Each o As Integer In GetAllStuff()
            Console.WriteLine(o.ToString())
        Next

        Console.WriteLine("done")
    End Sub

    Private Iterator Function GetAllStuff() As IEnumerable(Of Integer)
        Dim map As Dictionary(Of String, String) = New Dictionary(Of String, String)
        Dim tasks As New List(Of Integer)
        tasks.Add(1)

        For Each task As Integer In tasks
            Yield task
        Next

        'The value of map becomes null here under the new VB14 compiler in Release on .NET 4.6'
        For Each s As String In map.Values
            Yield 100
        Next
    End Function

End Module
  </file>
</compilation>

    Dim expectedOutput = <![CDATA[1
done]]>

    Dim compilation = CompilationUtils.CreateCompilationWithReferences(source, references:=LatestVbReferences, options:=TestOptions.DebugExe)
    CompileAndVerify(compilation, expectedOutput:=expectedOutput)
    CompileAndVerify(compilation.WithOptions(TestOptions.ReleaseExe), expectedOutput:=expectedOutput)
  End Sub

End Class

This may look like a convoluted mess due to XElement and XCData usage, but this is the format used by other Roslyn unit tests.

I only made one alteration to the code you posted in your question - that is replacing Console.ReadKey() with Console.WriteLine("done") so that I could track successful completion (as CompileAndVerify simply ignores exceptions).

The above test passes. There is no NullReferenceException on map.Values access, and the output is:

1
done

... as expected. It would, therefore, appear that your bug has been fixed - although whether or not the fix will be shipped with Visual Studio 2015 Update 2, I cannot tell.

The async variable capture issue was fixed by pull request #7693, but DataFlowPass.SetSlotUnassigned has been rewritten since (split into 2 methods and modified) so I cannot confirm whether the iterator problem you discovered was fixed by that specific pull request, or by some other code change.

like image 68
Kirill Shlenskiy Avatar answered Oct 23 '22 05:10

Kirill Shlenskiy