4 ms·
The code in question basically looks like this(C#): // Called from Thread 1. public void AddData(Data theData) { lock(dataQueue) {
by Measter 6y ago
The code in question basically looks like this(C#):
// Called from Thread 1.
public void AddData(Data theData) {
lock(dataQueue) {
dataQueue.Enqueue(theData);
}
}
// Worker for Thread 2.
private void ProcessingThread() {
while(true) {
int length = dataQueue.Count;
for (int i = 0; i < length; i++) {
Data curData;
lock(dataQueue) {
curData = dataQueue.Dequeue();
}
// Process data.
}
// Sleep for a short while.
}
}
And all of that is in its own class, where these are the only two functions that access the data queue.
So, from what I can see, in the event where the length is updated before the data is actually inserted, Thread 2 is still forced to wait until Thread 1 is finished before it can access it so it's still safe. Though if I'm wrong, please tell me. No one wants garbage data.
- gpderetta 6y agoI don't claim to know the c# memory model, but there is a chance that the compiler could hoist the 'int lenght = dataQueue.Count out of the loop. Also, it is possible that Count is not a member but an accessor and it might return garbage or crash if it sees some inconsistent data. Still, both of the above can be rare occurrences. The reason that you are not seeing no particular issues is because you are basically busy waiting and polling the queue (i.e. the sleep comment); if you replaced the dataQueue.Count with just a random number generator, the code will still work. If you were doing proper signaling instead of sleeping, then your code will get stuck in case of a missed wakeup.
- Measter 6y agoI'm not sure if it would be allowed to hoist `dataQueue.Count` out of the loop. The docs [0] say that multiple concurrent readers are supported, so I'm not sure if the compiler would be allowed to assume anything about the call's return value. You are correct in that `Count` is a property, and it could be doing extra things, but according to the implementation [1] it just returns the value of a private integer. Of course, this is an implementation detail and shouldn't be relied upon, but I'm not sure how much that could be changed. However, you are mistaken about the code continuing to work if `dataQueue.Count` were replaced with an RNG, or if it were otherwise higher than it should be. The `Dequeue` function raises an exception if called on an empty queue. This one is part of the API, not an implementation detail. An uncaught exception here would crash the thread, not just blindly carry on. Of course, I'm no expert, and could just be talking out of my ass. [0] https://docs.microsoft.com/en-gb/dotnet/api/system.collections.generic.queue-1?view=netcore-3.1#thread-safety https://docs.microsoft.com/en-gb/dotnet/api/system.collectio... [1] https://github.com/dotnet/runtime/blob/master/src/libraries/System.Collections/src/System/Collections/Generic/Queue.cs https://github.com/dotnet/runtime/blob/master/src/libraries/...
- gpderetta 6y agoConcurrent readers are supported, but not concurrent writers and readers. As Count is getting called outside of the critical section, the compiler can assume that there are no concurrent writers. Admittedly, the following critical section would make it hard for the compiler to actually apply any optimization in practice. Count is implemented as a simple read currently, but that's not guaranteed to be the case in the future (that's the whole reason for having a property. Fair enough about the rng, I'm not really a c# programmer, so I just assumed that dequeue would return a null on an empty queue. I'm not an expert either, and certainly not a c# expert (although I kind of like the language, I haven't written a line in 7 years), so YMMV.