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.

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.

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.