Search code examples
c#sql-server-2008concurrencyinfinite-loop

Multiple Users Simultaneously Press Button, Makes Infinite Loop Ineffective on C#


I have a c# infinite loop code on a WinForm project, which uses a value in SQL which alternates between 1/0 and if it's 0 it'll continue the process, while if it's 1 it'll stay in the loop. The purpose of this code is to prevent more than one person processing X.

My problem is that if I have multiple users using the program, and some of them press the button to run the process at the same time, the loop doesn't manage to catch that there's already someone processing X.

I figure the problem is that all users are reading the SQL value before any one of them has a chance to update the value in SQL to 1, and prevent other users from proceeding with the process.

Is there a way to fix this?

Here's a sample code of the loop:

SqlDataReader VerifyIfBusyReader = null;
                
                SqlCommand VerifyIfBusy = new SqlCommand("SELECT [Value] FROM [Variables] WHERE [TYPE] = 'IsBusy'", CONNECTION);
                int verify = 1;
                int counts = 0;
                int milliseconds = 2000;
                while (verify == 1)
                {
                    counts++;
                    if (counts == 30)
                    {
                        break;
                    }

                    VerifyIfBusyReader = VerifyIfBusy.ExecuteReader();
                    VerifyIfBusyReader.Read();
                    if (int.Parse(VerifyIfBusyReader[0].ToString()) == 1)
                    {
                        VerifyIfBusyReader.Close();
                    }
                    else if (int.Parse(VerifyIfBusyReader[0].ToString()) == 0)
                    {
                        verify = 0;
                        VerifyIfBusyReader.Close();
                    }
                    
                    Thread.Sleep(milliseconds);
                }

                SqlCommand IsBusyNow = new SqlCommand("UPDATE [Variables] SET [VALUE] = 1 WHERE [TYPE] = 'IsBusy'", CONNECTION);
                VerifyIfBusyReader = IsBusyNow.ExecuteReader();
                VerifyIfBusyReader.Close();

Any help is appreciated.

Of course, I have another function that updates SQL back to 0 once it finishes, but that's not the issue here.


Solution

  • You have a race condition in between getting a not-busy result and setting it.

    You can instead use an UPDATE with an OUTPUT, which will be atomic. Just loop it until you get a result.

    Note that if you are not supposed to get a resultset then you should use ExecuteNonQuery, and for a single result you can use ExecuteScalar.

    • Use async where possible
    • Make sure to dispose with using.
    • You don't need to store the result and loop, you can break the loop immediately.
    • Use proper for looping, rather than while.
    using (var conn = new SqlConnection(ConnStringHere))
    {
        using SqlCommand SetBusyOrNull = new SqlCommand(@"
    UPDATE Variables
    SET VALUE = 1
    OUTPUT inserted.VALUE
    WHERE TYPE = 'IsBusy'
      AND VALUE = 0;
    ", conn);
    
        int counts = 30;
        int milliseconds = 2000;
        for (var i = 0; i < counts; i++)
        {
            conn.Open();
            var isSet = (SetBusyOrNull.ExecuteScalar()) as int? == 1;
            conn.Close();
            if (isSet)
                return true;
    
            Thread.Sleep(milliseconds);
        }
        throw new Exception($"Still busy after {counts} tries");
        // alternatively return false;
    }