Re: [Boost-users] [thread] synchronized_value: value and move semantics

Subject: Re: [Boost-users] [thread] synchronized_value: value and move semantics
From: Klaim - Joël Lamotte (mjklaim_at_[hidden])
Date: 2013-06-08 12:19:52

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_at_[hidden]> 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