CODEWITHSUNDEEP
c#multithreading
Ben
Ben

Reputation: 174

Confused by variable evaluation in parent thread

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.

Upvotes: 1

Views: 84

Answers (1)

Cory Nelson
Cory Nelson

Reputation: 30001

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))

Upvotes: 3

Related Questions