Wednesday, August 28, 2013

STL is not thread safe, get over it

STL is not thread safe, get over it. Why it should be after all? STL is plain C++ code, compiler doesn't even know the existence of the STL, it's just C++ code released with your compiler, what the STL has in common with the C++ language is the fact that it's standardized. Given the fact that C++ and STL is standardized you can expect to have STL implemented on every platform with same guarantees enforced by the standard. You are not even obliged to use the STL deployed with your compiler indeed there are various version out there, see the roguewave ones for example (http://www.roguewave.com).
Let's take as example the std::list, let's suppose for a moment that the stl implementation it's thread safe,
some problem arise:

  1. If it's not needed to be thread safe you will get extra not needed overhead
  2. What shall do a thread calling a std::list::pop_back on an empty list?
    • Waiting for a std::list::push?
    • Returning with an "error"?
    • Throwing an exception?
    • Waiting for a certain ammount of seconds that an entry is available?
  3. It should be used in an enviroment with a single producer/single consumer, in this case it's possible to implement it without locks.
  4. Shall be multiple readers permitted?
Yes sure you can solve all the points above with a policy template, but just imagine your users complains.

Well, again, get over it, STL standardized is not thread safe you have to create your thread safe list embedding a real std::list, after all making a wrapper around an STL container is a so easy exercise that if you find difficult to do it yourself then you have to ask: "Am I ready to start with multithreading programming" even if someone provides me an out of the shelf std::list thread safe?
Consider that two different instance of  STL containers can be safely manipulated by different threads.

PS: I have written about std::list instead of the most widely used std::vector because std::vector for his own characteristics has to be used in a more "static" way with respect to an std::list and using an std::vector as a container used by multiple threads (producer/consumer) is a plain wrong choice.  






Thursday, June 13, 2013

Code inspecting Stroustrup (TC++PL4) (2nd issue)

Since I learned C++ reading one of the first edition of TC++PL and even if I'm programming using C++ since 2000 or so I'm reading the new TC++PL4 carefully as if this language is totally new to me. It seems my last post about an error found on this book will be not the unique and here we are again.
In 5.3.4.1 he introduces conditions and how two threads can interact each other communicating using events, it's presented the classical producer / consumer interaction exchanging Messages trough a queue, and this is the poor implementation proposed:

  class Message {
    // ...
  };

  queue mqueue;
  condition_variable mcond;
  mutex mmutex;

  void consumer() 
  {
     while(true) {
        unique_lock lck{mmutex};
        while (mcond.wait(lck)) /* do nothing */;
        
        auto m = mqueue.front();
        mqueue.pop();
        lck.unlock();
        // ... process m...
     }
  }

  void producer() 
  {
     while(true) {
        Message m;
        // ... fill the message ...
        unique_lock lck{mmutex};
        mqueue.push(m);
        mcond.notify_one();
     }
  }

This implementation is affected by at least three issues:

  • Unless very lucky the queue will grow indefinitely: that's because basically the consumer will wait at each cycle even if queue contains something, at the same time it has a chance (I repeat a "chance") to exit from the  condition_variable::wait() only each time the producer puts something in the queue.
  • The consumer can miss the condition_variable::notify_one event, indeed if the producer does the notify_one() but the other thread hasn't yet executed the wait() the consumer will block for no reason
  • The producer holds the unique_lock for more time than needed, the mutex has to only protect the queue not the condition as well
Let see how those producer / consumer should have been implemented:


  void consumer() 
  {
     while(true) {
        unique_lock lck{mmutex};
        while (mqueue.empty()) { // the empty condition has to be recheck indeed the thread
                                               // can get sporadic wakeup without any thread doing a notify
          mcond.wait(lck);
        }
        auto m = mqueue.front();
        mqueue.pop();
        lck.unlock();
        // ... process m...
     }
  }

  void producer() 
  {
     while(true) {
        Message m;
        // ... fill the message ...
        {
           unique_lock lck{mmutex};
           mqueue.push(m);
        }  // This extra scope is in here to release the mmutex asap
         mcond.notify_one();
     }
  }

In my opinion this should have been the version of the producer/consumer in TC++PL4, as you can see
with a simple extra scope and the right while(...) the issues reported in the bullets are solved.

There is another problem, I have to admit that this issue most of the times is a minore one:
  • The producer can issue notify_one() even if not needed, and this can be a performance issue 
to address it the producer has to "forecast" if the consumer can be in a blocked status, and this can happen only if after having acquired the mmutex the queue is empty, this is the final version of producer:

   void producer() 
  {
     while(true) {
        Message m;
        bool notifyIsNeeded = false;
        // ... fill the message ...
        {
           unique_lock lck{mmutex};
           if (mqueue.empty()) {
             notifyIsNeeded  = true;
           }
           mqueue.push(m);
        }  // This extra scope is in here to release the mmutex asap
        if ( notifyIsNeeded ) {
           mcond.notify_one();
        }
     }
  }

Writing correct code is not easy and writing correct multi-threaded is damn hard.



Sunday, June 9, 2013

Code inspecting Stroustrup (TC++PL4)

TC++PL4 is now on my desk and carefully reading it I have to say that even Stroustrup makes stupid mistakes in his classes implementations.

He illustrates a typical Vector implementation (pag. 73):

    class Vector {
    private:
        double* elem;
        int sz;
    public:
        Vector(int s);
        ~Vector() { delete [] elem; }
        
        Vector(const Vector& a);
        Vector& operator=(const Vector& a);

        double& operator[](int i);
        const double& operator[](int i) const;

        int size() const;
    };

and given the fact this class needs a copy constructor implemented he "implements" it (pag. 74):

     Vector::Vector(const Vector& a)
         :elem{new double[sz]},
          sz{a.sz}
     {
         for (int i = 0; i != sz; ++i)
         elem[i] = a.elem[i];
     }


as you can see the elem vector is built with a size retrieved from a not yet initialized variable, being it defined at page 74 I had my last hope flipping the page and checking at page 73 if "int sz" was declared before "double* elem" but it was not the case.

I'm sad.