[thread] synchronized_value: value and move semantics
Hi, boost::synchronized_value (not released yet [1][2][3]) is based on [4]. See below part of this paper adapted to the Boost.Thread interface. Currently boost::synchronized_value is in addition Copyable and Swappable. I was wondering if it is worth adding value and move semantics to synchronized_value. Making it EqualityComparable, LessThanComparable and Movable if the underlying type satisfy these requirements will allow to store them on a Boost.Container/C++11 container. Do you see something completely wrong with this addition? Has some of you had a need for this? Could you share the context? Best, Vicente [1] https://svn.boost.org/svn/boost/trunk/boost/thread/synchronized_value.hpp [2] https://svn.boost.org/svn/boost/trunk/libs/thread/example/synchronized_value... [3] https://svn.boost.org/svn/boost/trunk/libs/thread/example/synchronized_perso... [4] "Enforcing Correct Mutex Usage with Synchronized Values" by Anthony Williams http://www.drdobbs.com/cpp/enforcing-correct-mutex-usage-with-synch/22520026... ------------- [section The Problem with Mutexes] The key problem with protecting shared data with a mutex is that there is no easy way to associate the mutex with the data. It is thus relatively easy to accidentally write code that fails to lock the right mutex - or even locks the wrong mutex - and the compiler will not help you. std::mutex m1; int value1; std::mutex m2; int value2; int readValue1() { boost::lock_guard<boost::mutex> lk(m1); return value1; } int readValue2() { boost::lock_guard<boost::mutex> lk(m1); // oops: wrong mutex return value2; } Moreover, managing the mutex lock also clutters the source code, making it harder to see what is really going on. The use of synchronized_value solves both these problems - the mutex is intimately tied to the value, so you cannot access it without a lock, and yet access semantics are still straightforward. For simple accesses, synchronized_value behaves like a pointer-to-T; for example: boost::synchronized_value<std::string> value3; std::string readValue3() { return *value3; } void setValue3(std::string const& newVal) { *value3=newVal; } void appendToValue3(std::string const& extra) { value3->append(extra); } Both forms of pointer dereference return a proxy object rather than a real reference, to ensure that the lock on the mutex is held across the assignment or method call, but this is transparent to the user. [endsect] [/The Problem with Mutexes] [section Beyond Simple Accesses] The pointer-like semantics work very well for simple accesses such as assignment and calls to member functions. However, sometimes you need to perform an operation that requires multiple accesses under protection of the same lock, and that's what the synchronize() method provides. By calling synchronize() you obtain an strict_lock_ptr object that holds a lock on the mutex protecting the data, and which can be used to access the protected data. The lock is held until the strict_lock_ptr object is destroyed, so you can safely perform multi-part operations. The strict_lock_ptr object also acts as a pointer-to-T, just like synchronized_value does, but this time the lock is already held. For example, the following function adds a trailing slash to a path held in a synchronized_value. The use of the strict_lock_ptr object ensures that the string hasn't changed in between the query and the update. void addTrailingSlashIfMissing(boost::synchronized_value<std::string> & path) { boost::strict_lock_ptr<std::string> u=path.synchronize(); if(u->empty() || (*u->rbegin()!='/')) { *u+='/'; } } [endsect] [/Beyond Simple Accesses] [section Operations Across Multiple Objects] Though synchronized_value works very well for protecting a single object of type T, nothing that we've seen so far solves the problem of operations that require atomic access to multiple objects unless those objects can be combined within a single structure protected by a single mutex. One way to protect access to two synchronized_value objects is to construct a strict_lock_ptr for each object and use those to access the respective protected values; for instance: synchronized_value<std::queue<MessageType> > q1,q2; void transferMessage() { strict_lock_ptr<std::queue<MessageType> > u1 = q1.synchronize(); strict_lock_ptr<std::queue<MessageType> > u2 = q2.synchronize(); if(!u1->empty()) { u2->push_back(u1->front()); u1->pop_front(); } } This works well in some scenarios, but not all - if the same two objects are updated together in different sections of code then you need to take care to ensure that the strict_lock_ptr objects are constructed in the same sequence in all cases, otherwise you have the potential for deadlock. This is just the same as when acquiring any two mutexes. In order to be able to use the dead-lock free lock algorithms we need to use instead unique_lock_ptr, which is Lockable. synchronized_value<std::queue<MessageType> > q1,q2; void transferMessage() { unique_lock_ptr<std::queue<MessageType> > u1 = q1.unique_synchronize(boost::defer_lock); unique_lock_ptr<std::queue<MessageType> > u2 = q2.unique_synchronize(boost::defer_lock); boost::lock(u1,u2); // dead-lock free algorithm if(!u1->empty()) { u2->push_back(u1->front()); u1->pop_front(); } } [endsect] [/Operations Across Multiple Objects]
I just discovered synchronized_value from reading the 1.54.0 beta documentation. On Wed, Feb 20, 2013 at 7:02 PM, Vicente J. Botet Escriba < vicente.botet@wanadoo.fr> wrote:
Do you see something completely wrong with this addition?
No idea yet but I'm willing to test it in production code. See below for minor details.
Has some of you had a need for this? Could you share the context?
Yes, I think so. I have some cases (which I'm still working on but will be published as OSS soon) where I do something like that: struct ThingInfo { Id<ObjectInfo> id; std::string name; URI location; }; struct ThingContent { std::vector< Id<Foo> > foo_id_list; std::vector< Id<Bar> > bar_id_list; }; // this type MUST have thread-safe interface class Thing { public: explicit Thing( ThingInfo info ); ThingInfo info() const { boost::lock_guard<boost::mutex> lg( m_info_mutex ); return m_info; } ThingContent content() const { boost::lock_guard<boost::mutex> lg( m_content_mutex ); return m_content; } // + several modifying functions that use the work queue like that: void add( std::shared<Bar> bar ) { m_work_queue.push( [this, bar]{ m_bars.emplace_back( bar ); { boost::lock_guard<boost::mutex> lg( m_content_mutex ); m_content.bar_id_list.emplace_back( bar->id() ); } }); } void rename( std::string new_name ) { m_work_queue.push( [this, new_name]{ boost::lock_guard<boost::mutex> lg( m_content_mutex ); // SILENT ERROR!!! m_info.name = new_name; }); } private: WorkQueue m_work_queue; // tasks executed later std::vector< std::shared<Foo> > m_foos; // manipulated only by code in the work queue std::vector< std::shared<Bar> > m_bars; // idem ThingInfo m_info; // should be manipulated only with m_info_mutex locked ThingContent m_content; // should be manipulated only with m_content_mutex locked boost::mutex m_info_mutex; // protect m_info boost::mutex m_content_mutex; // protect m_content }; This is not real code but just how it looks like in my work-in-progress classes in my OSS project.
From reading the documentation, using synchronized_value I would then change the Thing implementation to:
// this type MUST have thread-safe interface class Thing { public: explicit Thing( ThingInfo info ); ThingInfo info() const { return *m_info; } ThingContent content() const { *return m_content; } // + several modifying functions that use the work queue like that: void add( std::shared<Bar> bar ) { m_work_queue.push( [this, bar]{ m_bars.emplace_back( bar ); m_content.synchronize()->bar_id_list.emplace_back( bar->id() ); }); } void rename( std::string new_name ) { m_work_queue.push( [this, new_name]{ m_info.synchronize()->name = new_name; // OK: CAN'T USE THE WRONG MUTEX!! }); } private: WorkQueue m_work_queue; // tasks executed later std::vector< std::shared<Foo> > m_foos; // manipulated only by code in the work queue std::vector< std::shared<Bar> > m_bars; // idem boost::sychronized_value<ThingInfo> m_info; boost::sychronized_value<ThingContent> m_content; }; This version is to me: - more explicit on reading; - shorter; - avoid some mistakes like the one pointed in the comments - which might be hard to spot; The only thing that bother me is that synchronized_value is a long name and the "_value" part is, in my opinion, too much. Also, maybe using the pointer-semantic operators is not the best idea. I guess that if std::optional does, then it's ok. Once Boost 1.54.0 is released I will have the opportunity to try it by refactoring my code. I'll report if I find issues. Joel Lamotte
Looks like I could have wrote: void add( std::shared<Bar> bar ) { m_work_queue.push( [this, bar]{ m_bars.emplace_back( bar ); m_content->bar_id_list.emplace_back( bar->id() ); // simplified }); } void rename( std::string new_name ) { m_work_queue.push( [this, new_name]{ m_info->name = new_name; // simplified }); } Which is even better. Now I have a question: Should the current (1.54.0 beta) version of synchronized_value work with multiple-readers-single-writer mutexes? Joel Lamotte
Le 08/06/13 18:48, Klaim - Joël Lamotte a écrit :
Now I have a question: Should the current (1.54.0 beta) version of synchronized_value work with multiple-readers-single-writer mutexes?
No. synchronized_value doesn't takes in account shared mutexes. Please create a ticket if you find the feature useful. Best, Vicente
On Sat, Jun 8, 2013 at 7:27 PM, Vicente J. Botet Escriba < vicente.botet@wanadoo.fr> wrote:
No. synchronized_value doesn't takes in account shared mutexes. Please create a ticket if you find the feature useful.
I'm not sure yet because I'm not in a position to check if there would be a performance impact in my use cases by replacing mutexes (as shown in the example) by shared mutexes and associated locks. I suspect that there will be tens of concurrent info() calls while m_info is modified, but I don't know if it's worth using a shared_mutex for such number of accesses. I will have to measure before getting back to this point. A bit of bikesheding: I like synched<T> as a shortcut name (but I don't think that's valid english). Joel Lamotte
participants (2)
-
Klaim - Joël Lamotte
-
Vicente J. Botet Escriba