1

I am using the BlockingQueue code posted in this question, but realized I needed to use a Stack instead of a Queue given how my program runs. I converted it to use a Stack and renamed the class as needed. For performance I removed locking in Push, since my producer code is single threaded.

My problem is how can thread working on the (now) thread safe Stack know when it is empty. Even if I add another thread safe wrapper around Count that locks on the underlying collection like Push and Pop do, I still run into the race condition that access Count and then Pop are not atomic.

Possible solutions as I see them (which is preferred and am I missing any that would work better?):

  1. Consumer threads catch the InvalidOperationException thrown by Pop().
  2. Pop() return a nullptr when _stack->Count == 0, however C++-CLI does not have the default() operator ala C#.
  3. Pop() returns a boolean and uses an output parameter to return the popped element.

Here is the code I am using right now:

generic <typename T>
public ref class ThreadSafeStack
{
public:
  ThreadSafeStack()
  {
    _stack = gcnew Collections::Generic::Stack<T>();
  }

public:
  void Push(T element)
  {
    _stack->Push(element);
  }

  T Pop(void)
  {
    System::Threading::Monitor::Enter(_stack);
    try {
      return _stack->Pop();
    }
    finally {
      System::Threading::Monitor::Exit(_stack);
    }
  }

public:
  property int Count {
    int get(void)
    {
      System::Threading::Monitor::Enter(_stack);
      try {
        return _stack->Count;
      }
      finally {
        System::Threading::Monitor::Exit(_stack);
      }
    }
  }

private:
  Collections::Generic::Stack<T> ^_stack;
};
Community
  • 1
  • 1
mindless.panda
  • 4,014
  • 4
  • 35
  • 57
  • "For performance I removed locking in Push, since my producer code is single threaded." What do you mean by this? You only have one producer? Is your producer code running at the same time as consumers? – tony Mar 15 '10 at 21:04
  • Yes, I only have one producer and no my producer code does not run at same time as consumers. It runs first and then runs the multiple consumers with the ThreadSafeStack produced by the producer. – mindless.panda Mar 15 '10 at 21:38

2 Answers2

4

Personally, I would use your option 3., but rename this TryPop().

This would make it behave more like the Framework's ConcurrentQueue<T>.TryDequeue (in .NET 4).

Edit:

I'd declare it like so:

public:
bool TryPop([Out] T% result);

In your implementation, you just set the T value in your method body...

Reed Copsey
  • 554,122
  • 78
  • 1,158
  • 1,373
  • Not at all familiar with how to use out parameters in c++-cli, any links or sample code? – mindless.panda Mar 15 '10 at 17:11
  • Given that the TryPop should return a nullptr if stack is empty, do you recommend passing in a nullptr where I call TryPop or ensuring I set the out parameter to a nullptr? Again, I am not sure how to do the c# equivalent of default(T). I found this: http://stackoverflow.com/questions/1962600/what-is-the-c-cli-equivalent-to-cs-defaultt – mindless.panda Mar 15 '10 at 17:30
  • 1
    @user260197: My C++/CLI is a bit rusty, but I think you just do: result = T(); – Reed Copsey Mar 15 '10 at 17:43
2

Option #3 is the way to go and Marc Gravell has posted an excellent implementation of a BufferedQueue/BlockingQueue which he called SizeQueue:

Creating a blocking Queue<T> in .NET?

Given Marc's queue example it should be pretty easy to swap in a stack and it would work in a similar fashion.

Community
  • 1
  • 1
Kiril
  • 39,672
  • 31
  • 167
  • 226