1

If my use case requires that I may get the old state of the object or the new state, but not a corrupted/inconsistent state, should I still be locking the objects ?

e.g.

Public Class Class1
    Private StopFlag As Boolean = False
    Private MyQueue As New Queue(Of Something)

    Public Sub AddItem(item As Something)
        MyQueue.Enqueue(item)
    End Sub

    Public Sub Start()
        DoWork()
    End Sub

    Public Sub [Stop]()
        StopFlag = True
    End Sub

    Private Sub DoWork()
        While Not StopFlag
            If MyQueue.Count > 0 Then
                Dim item As Something = MyQueue.Dequeue
                'do something with this item here.. 
                'I have just put sleep to simulate time spent...
                Threading.Thread.Sleep(1000)
            End If
            Threading.Thread.Sleep(100)
        End While
    End Sub
End Class

Public Class Something
    'stub
End Class

In the code above, there are two candidates for lock - StopFlag and MyQueue.

Assume that user is continuously enqueuing items at approximately 0.5 to 1 sec. interval. It really doesn't matter to me whether someone was adding to the queue, when I was trying to dequeue from it (except for a lag of 100ms which is acceptable). This is because the item will get processed in the next loop cycle anyways.

For the same reason, the StopFlag state doesn't matter. It just may process one or two extra cycles, which is acceptable. This class is part of a service which is rarely stopped, except for maintenance. So putting locks to catch once-in-a-while event seems just like an overkill.

Do I still necessarily need to lock both these objects? Will avoiding use of locks cause problems in this case?

In other words, can my StopFlag ever have anything other than True or False? And can the MyQueue.Dequeue ever result into an object which is not a valid instance of Something class?

Pradeep Kumar
  • 6,836
  • 4
  • 21
  • 47

2 Answers2

2

can my StopFlag ever have anything other than True or False?

No, it will only ever have True or False, because assigning to a boolean variable is an atomic operation.

And can the MyQueue.Dequeue ever result into an object which is not a valid instance of Something class?

No, this won't happen either. The reason is because the Queue never manipulates the internal state of the Something. It only passes around a pointer to a valid Something instance.

If my use case requires that I may get the old state of the object or the new state, but not a corrupted/inconsistent state, should I still be locking the objects?

Yes! You absolutely should! And this applies to both your StopFlag boolean flag and MyQueue instance.

StopFlag

For the same reason, the StopFlag state doesn't matter. It just may process one or two extra cycles, which is acceptable.

The problem is that without any sort of memory synchronization (which locking does for you), there is no guarantee that the thread in the loop will ever see an up-to-date value for StopFlag. So, though unlikely in practice, without locking (or some other form of memory synchronization), it is entirely possible that the StopFlag will never be evaluated as True in the loop condition.

For a more thorough explanation about this, have a look at the following threads:

... and there are many more if you take the time to search for them.

Queue

Without any locking, there are definitely many ways that you can corrupt the internal state of your Queue or have it throw exceptions on you.

Here is a very simple example that illustrates how easily this can happen.

The following is the source code for Queue's Enqueue method (which you can have a look here if you want). It's in C#, but I'm sure it won't be too hard to grasp what's going on.

public void Enqueue(T item) {
    if (_size == _array.Length) {
        int newcapacity = (int)((long)_array.Length * (long)_GrowFactor / 100);
        if (newcapacity < _array.Length + _MinimumGrow) {
            newcapacity = _array.Length + _MinimumGrow;
        }
        SetCapacity(newcapacity);
    }

    _array[_tail] = item;
    _tail = (_tail + 1) % _array.Length;
    _size++;
    _version++;
}

Using the above as reference, what do you suppose would happen if 2 threads executed the following line at pretty much the exact same time (which is entirely possible without any locking). So something like this, in this order:

  1. Thread 1: _array[_tail] = item;
  2. Thread 2: _array[_tail] = item;
  3. Thread 1: _tail = (_tail + 1) % _array.Length;
  4. Thread 2: _tail = (_tail + 1) % _array.Length;

If you take the time to think about that, you'll see that thread 2 will accidentally overwrite the item that thread 1 enqueued. So that item is now lost.

Additionally, the _tail pointer gets moved two positions, even though only one array slot was used. That introduces other kinds of corruption as well.

And this is a very oversimplified example, of which there are many more. And this is without even talking about how, when you're multithreading, without locking, the CPU is free to reorder the instructions in a funny and unexpected way, and that creates other problems that are very hard to anticipate.

Conclusion

Don't sacrifice correctness to gain a few milliseconds, if that. It's not worth it.

Multi-threading is a very complicated subject that is very hard to get right to begin with. Trying to be clever on top of that by taking performance shortcuts is a recipe for disaster. Again: not worth it.

Community
  • 1
  • 1
sstan
  • 35,425
  • 6
  • 48
  • 66
1

The resource is what has been given, the documentation. Here is some code that shows what happens. I manipulated the timers to show how it can fail. You will need a form with two buttons

Public Class Form1

    Dim testClass1 As New Class1
    Dim eq As Threading.Thread
    Dim dq As Threading.Thread

    Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
        Button1.Enabled = False

        eq = New Threading.Thread(AddressOf eqThrd)
        eq.IsBackground = True
        eq.Start()
        dq = New Threading.Thread(AddressOf dqThrd)
        dq.IsBackground = True
        dq.Start()
    End Sub

    Private Sub eqThrd()
        Do While Not testClass1.StopFlag
            Try
                testClass1.AddItem(New Something)
            Catch ex As Exception
                'Source array was not long enough. Check srcIndex and length, and the array's lower bounds.
                'or
                'System.OutOfMemoryException' occurred in System.dll
                Debug.WriteLine(ex.Message)
                Stop
            End Try
            Threading.Thread.Sleep(0)
        Loop
    End Sub

    Private Sub dqThrd()
        testClass1.Start()
    End Sub

    Private Sub Button2_Click(sender As Object, e As EventArgs) Handles Button2.Click
        testClass1.Stop()
    End Sub
End Class

Public Class Class1
    Public StopFlag As Boolean = False
    Private MyQueue As New Queue(Of Something)

    Public Sub AddItem(item As Something)
        MyQueue.Enqueue(item)
    End Sub

    Public Sub Start()
        DoWork()
    End Sub

    Public Sub [Stop]()
        StopFlag = True
    End Sub

    Private Sub DoWork()
        While Not StopFlag
            If MyQueue.Count > 0 Then
                Dim item As Something = MyQueue.Dequeue
                'do something with this item here.. 
                'I have just put sleep to simulate time spent...
                Threading.Thread.Sleep(0)
            End If
            Threading.Thread.Sleep(100)
        End While
    End Sub
End Class

Public Class Something
    'stub
End Class
dbasnett
  • 11,334
  • 2
  • 25
  • 33
  • This was really helpful. Thanks :) – Pradeep Kumar Sep 25 '15 at 12:54
  • @PradeepKumar - anything that MIGHT be touched by more than one thread has to be protected. This is just one way. The UI protects controls by marshaling all control changes to the UI thread. – dbasnett Sep 25 '15 at 22:35