-1

I have a class to hold a stack of strings to paint to a UI with which I want to block if there are no items, and take LIFO if there are a lot of items.

internal class PaintStack
{
    private BlockingCollection<PaintString> paintStack;

    public PaintStack()
    {
        paintStack = new BlockingCollection<PaintString>(new ConcurrentStack<PaintString>());
    }

    public void Add(PaintString p)
    {
        paintStack.Add(p);
    }

    public PaintString Take()
    {
        return paintStack.Take();
    }
}

When I add a new object to the collection I am seeing all objects as the same as the new object. What's going on?

New strings come in with price value of 1450.4

new price

Now the whole concurrent stack has a price of 1450.4

stack

Then next string comes in with price value of 1460.4

new price

The whole stack appears to have a value of 1460.4

stack

What is going wrong?

rupweb
  • 3,052
  • 1
  • 30
  • 57
  • 1
    why not use `BlockingCollection` directly? there should be some extra useful members in PaintStack besides wrapping `BlockingCollection` – Lei Yang Mar 15 '22 at 15:21
  • because of [this](https://stackoverflow.com/questions/5001003/fast-and-best-producer-consumer-queue-technique-blockingcollection-vs-concurrent) whereby I can use the FIFO nature of a concurrent stack along with the `block` of the blocking collection. It's basically a kind of Java [linked blocking queue](https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/LinkedBlockingQueue.html) – rupweb Mar 15 '22 at 15:22
  • 1
    I'd suspect `PaintString` rather than the collection, tbh. BTW, "FIFO" would be a queue, a Stack is "LIFO". – Fildor Mar 15 '22 at 15:24
  • Is there anything wrong with this implementation `paintStack = new BlockingCollection(new ConcurrentStack());` ? it's not like `new` the stack for every new element?!! – rupweb Mar 15 '22 at 15:24
  • 1
    _"Is there anything wrong with this implementation"_ - Nothing obvious. You are just setting the backing DataStructure to be a `ConcurrentStack` - which is ok if you want the behavior of that specific DataStructure. – Fildor Mar 15 '22 at 15:25
  • Have you checked that there is an actual issue and not debugger visualization bug? – Guru Stron Mar 15 '22 at 15:30
  • 1
    I was not able to [repro](https://sharplab.io/#v2:C4LgTgrgdgNAJiA1AHwAICYAMBYAUBgRj1UwAJUCA6AYQHsAbegUwGNgBLWqAZxq5YhgwTKMADceUlOkypANwCGYUi1IBeUlCYB3UgCF6tFgGt2UAOZ1GrDlwA8ABQVngAZWBgz5gHwAKLbp0UAJCIm7ACiaOzqLunhZ+AJSJEriSshkslACCcHD+OqROLnFeAN4AknDqpAQAvinpGTJZufkBRTHh8eaV1RoEmA2pzbKteQW6xbEe5VU16MNNo6TaABbszKS+WQAqYACeuwrGTL60EMCkisoAHsnLo2WPK1IUAJy+t5RVjbivUjqxAInwARIAeDcAIPugv7EADM5HQnRKswsy2e/2aqARLlI8zKpHMTHEpG4xLEpCBmMBQA=) – Guru Stron Mar 15 '22 at 15:32
  • I meant to say LIFO... confusing eh?! – rupweb Mar 15 '22 at 15:38

1 Answers1

2

You are probably adding multiple references to the same object to the list. So when you change your object, everything in the list is affected, since they are all referring to the same object.

To solve this I would suggest using an immutable object or record. So instead of changing an existing object, you would create a new object with your requested changes. This neatly avoids issues like this, and helps ensuring thread-safety.

Also, as mentioned in the comment, a stack is Last In First Out (LIFO). If you want FIFO, use a ConcurrentQueue.

Also, you do not need concurrent* if you are not writing multithreaded code. My standard recommendation regarding multithreaded code is to only do it if you are familiar with the risks, and how to mitigate them. Multi threading is infamous for causing serious, hard to reproduce, bugs.

JonasH
  • 28,608
  • 2
  • 10
  • 23
  • Thanks, the problem was trying to re-use the same object by value which was clearly used by reference also. – rupweb Mar 15 '22 at 15:30