Confused by variable evaluation in parent thread

Question

I work for a company that has multiple stores out in the field. When we need to connect to a store POS system, we need to connect to their VPN first. I'm working on a GUI to help simplify the process. I've got it working just fine, but I stumbled across some behavior that I don't understand.

When the program connects to the VPN, it will fire off the OpenVPN console program, wait for the program to ask for the username and password and then monitor the programs output to detect if the connection was successful, track error messages or when the connection was dropped due to inactivity.

On my development machine, I was getting consistent and accurate results. When I ran it on one of the actual machines, it wouldn't properly detect when the VPN connection was successful. For example...

This is the code that monitors the OpenVPN output.

var outputProcessingThread = new Thread(() =>
{
    while (_trackOpenVpnOutput)
    {
        var buffer = new byte[256];
        IAsyncResult ar = _vpnProcess.StandardOutput.BaseStream.BeginRead(buffer, 0, 256, null, null);
        ar.AsyncWaitHandle.WaitOne();
        var bytesRead = _vpnProcess.StandardOutput.BaseStream.EndRead(ar);
        if (bytesRead > 0)
        {
            var outputString = Encoding.ASCII.GetString(buffer, 0, bytesRead);
            RaiseDebugVpnMessageEvent(new VpnMessageEventArgs(outputString, VpnMessageType.Info));
            if (outputString.Contains("Initialization Sequence Completed With Errors"))
            {
                _trackOpenVpnOutput = false;
                _vpnConnected = false;
                const string message = "Unable to connect to VPN: Errors in Initialization Sequence";
                RaiseVpnMessageEvent(new VpnMessageEventArgs(message, VpnMessageType.Error));
                return;
            }

            if (outputString.Contains("Initialization Sequence Completed"))
            {
                RaiseVpnMessageEvent(new VpnMessageEventArgs(String.Format("Connected to #{0}'s VPN.", storeNumber), VpnMessageType.Info));
                _vpnConnected = true;
            }
            else if (outputString.Contains("Inactivity timeout"))
            {
                RaiseVpnMessageEvent(new VpnMessageEventArgs("Inactivity timeout. Disconnected from VPN.", VpnMessageType.Warning));
                _vpnConnected = false;
                _trackOpenVpnOutput = false;
                return;
            }
            else if (outputString.Contains("TLS Error"))
            {
                RaiseVpnMessageEvent(new VpnMessageEventArgs("Unable to connect to VPN: TLS Error.", VpnMessageType.Error));
                _vpnConnected = false;
                _trackOpenVpnOutput = false;
                return;
            }
        }
    }
});

Then after I fire off that thread, I check it with this (wait until either the VPN is connected or their was an error and the output processing thread changed _trackOpenVpnOutput to false)...

while (!_vpnConnected && _trackOpenVpnOutput)
{
    Console.WriteLine("VPN Connected: {0} \t\tTracking OpenVPN: {1}", _vpnConnected, _trackOpenVpnOutput);
}

So the part that I don't understand, is that if I comment out the Console.WriteLine(--message--), then those variables don't seem to get evaluated properly in the while (!_vpnConnected && _trackOpenVpnOutput) loop.

For now, I'm going to leave the debugging Console.WriteLine in there since it works, but I would like to understand why I'm seeing this behavior and hopefully learn what the proper way to handle situations like these.

Consider using ildasm on the dll, just to see if there is a difference in the IL. Its possible that optimizations are made when the Console statement is taken away. Post any IL differences here in the question. — mrwaim, Dec 22 '14 at 21:30
Sounds like, at the minimum, you need some sort of memory barrier around `_vpnConnected` and `_trackOpenVpnOutput`. You could try declaring them [`volatile`](http://stackoverflow.com/questions/19382705/c-sharp-volatile-keyword-usage-vs-lock) but without seeing your outer code I'm not sure that is sufficient. — dbc, Dec 22 '14 at 21:31

score 3 · Accepted Answer · answered Dec 22 '14 at 21:32

3

I'm guessing that _vpnConnected _trackOpenVpnOutput are both bool fields.

What you've stumbled across is multithreaded change visibility and additionally, the compiler being able to optimize assuming zero visibility. The compiler isn't aware another thread is able to change those variables, so it is free to optimize out the repeated reads.

You need to tell it not to optimize out these reads -- to re-read the variable every time. You can do this by either marking the variables as volatile, or by using Volatile.Read:

while (!Volatile.Read(ref _vpnConnected)
     && Volatile.Read(ref _trackOpenVpnOutput))

answered Dec 22 '14 at 21:32

Cory Nelson

29,236
5
72
110

You're correct, they were both bools. I apologize for not stating that. I changed the declaration of those variables to be volatile and it worked correctly. Thank you very much for that information. I'm going to go read about the volatile keyword so I understand it better! – Ben Dec 22 '14 at 21:42
Recommend reading about the [C# memory model](http://msdn.microsoft.com/en-us/magazine/jj863136.aspx). – Cory Nelson Dec 22 '14 at 21:52

Confused by variable evaluation in parent thread

1 Answers1