C# Code optimization causes problems with Interlocked.Exchange()

Question

I have a frustrating problem with a bit of code and don't know why this problem occurs.

//
// .NET FRAMEWORK v4.6.2 Console App

static void Main( string[] args )
{
    var list = new List<string>{ "aa", "bbb", "cccccc", "dddddddd", "eeeeeeeeeeeeeeee", "fffff", "gg" };

    foreach( var item in list )
    {
        Progress( item );
    }
}

private static int _cursorLeft = -1;
private static int _cursorTop = -1;
public static void Progress( string value = null )
{
    lock( Console.Out )
    {
        if( !string.IsNullOrEmpty( value ) )
        {
            Console.Write( value );
            var left = Console.CursorLeft;
            var top = Console.CursorTop;
            Interlocked.Exchange( ref _cursorLeft, Console.CursorLeft );
            Interlocked.Exchange( ref _cursorTop, Console.CursorTop );
            Console.WriteLine();
            Console.WriteLine( "Left: {0} _ {1}", _cursorLeft, left );
            Console.WriteLine( "Top: {0} _ {1}", _cursorTop, top );
        }
    }
}

When running without Code optimization then the result is as expected. _cursorLeft and left as far as _cursorTop and top are equal.

aa
Left: 2 _ 2
Top: 0 _ 0
bbb
Left: 3 _ 3
Top: 3 _ 3

But when I run it with Code optimization both values _cursorLeft and _cursorTop become bizzare:

aa
Left: -65534 _ 2
Top: -65536 _ 0
bb
Left: -65533 _ 3
Top: -65533 _ 3

I found out 2 workarounds:

1. set _cursorLeft and _cursorTop to 0 instead of -1
2. let Interlocked.Exchange take the value from left resp. top

Because workaround #1 does not match my needs I ended up with workaround #2:

private static int _cursorLeft = -1;
private static int _cursorTop = -1;
public static void Progress( string value = null )
{
    lock( Console.Out )
    {
        if( !string.IsNullOrEmpty( value ) )
        {
            Console.Write( value );

            // OLD - does NOT work!
            //Interlocked.Exchange( ref _cursorLeft, Console.CursorLeft );
            //Interlocked.Exchange( ref _cursorTop, Console.CursorTop );

            // NEW - works great!
            var left = Console.CursorLeft;
            var top = Console.CursorTop;
            Interlocked.Exchange( ref _cursorLeft, left );  // new
            Interlocked.Exchange( ref _cursorTop, top );  // new
        }
    }
}

But where does this bizarre behavior comes from?
And is there a better workaround/solution?

---

[Edit by Matthew Watson: Adding simplified repro:]

class Program
{
    static void Main()
    {
        int actual = -1;
        Interlocked.Exchange(ref actual, Test.AlwaysReturnsZero);
        Console.WriteLine("Actual value: {0}, Expected 0", actual);
    }
}

static class Test
{
    static short zero;
    public static int AlwaysReturnsZero => zero;
}

---

[Edit by me:]
I figured out another even shorter example:

class Program
{
    private static int _intToExchange = -1;
    private static short _innerShort = 2;

    // [MethodImpl(MethodImplOptions.NoOptimization)]
    static void Main( string[] args )
    {
        var oldValue = Interlocked.Exchange(ref _intToExchange, _innerShort);
        Console.WriteLine( "It was:   {0}", oldValue );
        Console.WriteLine( "It is:    {0}", _intToExchange );
        Console.WriteLine( "Expected: {0}", _innerShort );
    }
}

Unless you don't use Optimization or set _intToExchange to a value in the range of ushort you would not recognize the problem.

Ironworks

I can reproduce this. – Soracco

I took the liberty of adding a simplified repro.You may incorporate it or delete it as you see fit. – Soracco

@MatthewWatson Good idea! I really thought it has to be a specific problem but it seems like a big bug. – Ironworks

I have reported this issue here: connect.microsoft.com/VisualStudio/feedback/details/3131687/… – Soracco

It should be fixed in the future: github.com/dotnet/coreclr/issues/10714 – Bambara

Answer 1

You diagnosed the problem correctly, this is an optimizer bug. It is specific to the 64-bit jitter (aka RyuJIT), the one that first started shipping in VS2015. You can only see it by looking at the generated machine code. Looks like this on my machine:

00000135  movsx       rcx,word ptr [rbp-7Ch]       ; Cursor.Left
0000013a  mov         r8,7FF9B92D4754h             ; ref _cursorLeft
00000144  xchg        cx,word ptr [r8]             ; Interlocked.Exchange

The XCHG instruction is wrong, it uses 16-bit operands (cx and word ptr). But the variable type requires 32-bit operands. As a consequence, the upper 16-bits of the variable remain at 0xffff, making the entire value negative.

Characterizing this bug is a bit tricky, it is not easy to isolate. Getting the Cursor.Left property getter inlined appears to be instrumental to trigger the bug, under the hood it accesses a 16-bit field. Apparently enough to, somehow, make the optimizer decide that a 16-bit exchange will get the job done. And the reason why your workaround code solved it, using 32-bit variables to store the Cursor.Left/Top properties bumps the optimizer into a good codepath.

The workaround in this case is a pretty simple one, beyond the one you found, you don't need Interlocked at all because the lock statement already makes the code thread-safe. Please report the bug at connect.microsoft.com, let me know if you don't want to take the time and I'll take care of it.

Anthrax

@MatthewWatson Okay – Ironworks

I was not able to reproduce this in .net core, did you try to do that? – Nedanedda

I didn't. Not likely to repro, making the Console class cross-platform so it can run on Linux and OSX surely prevents the property from being easy to inline. – Anthrax

But you can reproduce this issue with just method with return type int but which internally returns short, without any access to Console class at all. – Nedanedda

Answer 2

I don't have an exact explanation, but still would like to share my findings. It seems to be a bug in x64 jitter inlining in combination with Interlocked.Exchange which is implemented in native code. Here is a short version to reproduce, without using Console class.

class Program {
    private static int _intToExchange = -1;

    static void Main(string[] args) {
        _innerShort = 2;
        var left = GetShortAsInt();
        var oldLeft = Interlocked.Exchange(ref _intToExchange, GetShortAsInt());
        Console.WriteLine("Left: new {0} current {1} old {2}", _intToExchange, left, oldLeft);
        Console.ReadKey();
    }

    private static short _innerShort;
    static int GetShortAsInt() => _innerShort;
}

So we have an int field and a method which returns int but really returns 'short' (just like Console.LeftCursor does). If we compile this in release mode with optimizations AND for x64, it will output:

new -65534 current 2 old 65535

What happens is jitter inlines GetShortAsInt but doing so somehow incorrectly. I'm not really sure about why exactly things go wrong. EDIT: as Hans points out in his answer - optimizer uses incorrect xchg instuction in this case to perform as exchange.

If you change like this:

[MethodImpl(MethodImplOptions.NoInlining)]
static int GetShortAsInt() => _innerShort;

It will work as expected:

new 2 current 2 old -1

With non-negative values it seems to work at first site, but really does not - when _intToExchange exceeds ushort.MaxValue - it breaks again:

private static int _intToExchange = ushort.MaxValue + 2;
new 65538 current 2 old 1

So given all this - your workaround looks fine.

Nedanedda

Maybe it would be an good idea to check if this is still happening with .net core and then report it on github, because this seems like a really weird bug. – Bambara

Another "workaround" for your example is: static int GetShortAsInt() => Convert.ToInt32(_innerShort); – Ironworks

Okay, one more workaround: remove _innerShort = 2; and set private static short _innerShort = 2; – Ironworks

@Ironworks I specifically crafted this example for it to NOT work correctly :) – Nedanedda

@Nedanedda I absolutely know. I just wanted to report the strange behavior. – Ironworks

@Bambara I was not able to reproduce it in .net core. – Nedanedda

@Nedanedda you need to set <Optimize>true<Optimize> and <DebugType></DebugType> in the .csproj file to reproduce it – Bambara