VB.NET - Is this thread safe?

Question

Let's say that I have a module that has a Queue in it.

For other entities to Enqueue, they must go through a function:

public sub InsertIntoQueue(Obj)
    MyQueue.Enqueue(Obj)
end sub

If I have multiple threads running and they want to call InsertIntoQueue(), is this considered thread safe?

I am under the impression that there is only one copy of the instructions in memory necessary to perform the InsertIntoQueue() function... which would lead me to think that this is thread safe.

However, I wonder what happens when two threads attempt to run the function at the same time?

Is this thread safe, and if not, how can I make it thread safe? (and What would be the performance implications regarding speed and memory usage)

Thanks in advance for your time.

Answer 1

No this is not thread safe.

Public static (Shared in Visual Basic) members of this type are safe for multithreaded operations. Instance members are not guaranteed to be thread-safe.

From the MSDN Site.

I would suggest adding a object to represent the sync handle to your object

Dim SyncHandle as Object = new Object()

And modify your method as such

Public Sub InsertIntoQueue(Object item)
    SyncLock SyncHandle 
       MyQueue.Enqueue(item)
    End SyncLock
End Sub

Answer 2

Use Queue.Synchronized wrapper.

Answer 3

One set of instructions does not mean it is thread safe. As far as instructions go you always have only one set.

Now, looking at the code sample you supplied it is impossible to say whether it is thread safe or not. All standard .NET collections, including Queue are not thread safe by themselves, but give you access to a synchronized version of thmeselves.

Now in trems of performance, of course there is a performance hit, how big - it depends on the scope of the lock and some other things. In particular using global locks in a web app can become a serious bottleneck under heavy load

Answer 4

do

SyncLock MyQueue
   MyQueue.Enqueue(Obj)
End SyncLock

End Sub

Answer 5

At the risk of going slightly off the OP's topic, consideration should be given to the other methods of the queue. I assume that there is at least one thread dequeuing objects, and possibly you are also checking to see if the queue is empty?

From a dequeue perspective if you have more than one thread dequeuing, and they check to see if the queue is not empty before calling dequeue (to prevent the invalid operation exception) then it's entirely likely that one thread (thread a) could dequeue the last item in the queue, inbetween another thread (thread b) reading the that the queue isn't empty and it calling dequeue, thus thread a will cause the invalid operation exception.

You could put the lock around the check for empty and the dequeue to get around this.

This and this are interesting articles about thread safety, and also I can recommend reading this and/or this, whilst they aren't all in vb.net they explain threading in detail.

Answer 6

I'm not an expert on thread safety but I'm trying to learn as much as I can about that matter.

I used to think (like you) that this operation can be thread safety as are just writting to the queue data on different threads and no dequeue is being done. But as someone explained here (and is everywhere at the MSDN documentation):

Public static (Shared in Visual Basic) members of this type are safe for multithreaded operations. Instance members are not guaranteed to be thread-safe.

That means that maybe internally the MyQueue.Enqueue(Obj) is done like this:

1. Put data on Queue();
2. Augment Queue pointer;

If it is done in this way you will have threading problems because as you can see you can be overwriting the same position on the Queue with two threads because one is writing after other has did the same but that has not been able to increment the pointer already.

Having this in mind you have several options as stated here, locking on the Enqueue() method, using Queue.Synchronized() or maybe even simpler but with bigger impact on performance, locking on a private property when accessing the Queue object in this way (because anything you do with the Queue will be not thread safe):

Private ReadOnly Property MyQueue() as Queue
Get
    SyncLock (m_myQueueLock)
        Return m_myQueue
    EndSyncLock
End Get
End Property

Hope this helps!