On Thu, Jun 13, 2013 at 8:13 AM, Vicente J. Botet Escriba < vicente.botet@wanadoo.fr> wrote:
Just to clarify, and make sure I understand 100% correctly,
are you talking about the member's construction (m_info) or the owning object construction (Sequence)?
I meant the member.
I assume the later, so the member don't need locking until the end of the
constructor of the owning object.
No. Any access to the member would imply a lock after construction.
m_info-> returns a lock.
Ok then.
I suspect that there might be other cases like this one where
the object responsable for the synchronized object should be able, rarely, to access the object unsafely.
You are supposing that there is an object responsible for synchronized
object, but I don't know what this means and how to ensure it. If you want this pattern , you maybe need external locking. See the class externally_locked.
Ok I'll take a look.
For example?
This is just speculation. Maybe I'm just pessimistic. Currently I only have the case of the constructor and I don't think I will have others. I have another bigger code base which have more complex concurrent cases where I might find similar usage (access to a usually locked object in a case where I am 100% sure there can't be concurrent accesses).
Could you develop the case and explain how are you 100% sure that there is no concurrent access?
I'll get back to this after looking at the codebase I was talking about.
I will search and report if I find something, I'm not sure I will.
Suggestion: add an unsafe access to the object, in a very visible way.
m_info.unsafe_access().id( new_id );
OR
m_info.unsafe_access()->id( new_id ); // using a smart pointer which checks if safe accesses have been done inbetween and throw if it is the case? or any other checks detecting potential error
BTW, I don't know what "checks if safe accesses have been done
inbetween" could mean? Could you clarify?
I am not sure really, I don't think I have enough experience in the domain. I was thinking maybe there are ways to note if at least one lock have been taken while while accessing the object unsafely. Maybe just throw an exception on locking if an unsafe access is going on?
However it is still not a perfect solution. In the real code I have some logic I need to do before manipulating the synchronized member. It should be done in this order, first some logic, then use thesynched object. But even if I use a function to create the member but still have to access it later because of that logic, there is no point in doing that.
You can create a unsynchronized SequenceInfo object, do whatever you want with and last create the synchronized SequenceInfo just by copying.
Yes, when the object is copyable is less of a problem.
To clarify what I'm trying to explain (this is fictive code close to mine,
which is more complex):
explicit Sequence( SequenceId new_id, SequenceInfo info ) : m_info( merge_info( new_id, info ) ) // assuming I follow your suggestion { if( first_thing_to_do() ) { second_thing_to_do( m_info->name ); // still need to lock } }
I recognize that the factory function could be less natural, but it works
For this case I agree.
I'm using function name instead of real code just so you get the idea,
sorry if it's not real code. I try to do minimum work in the constructor but I noticed the locking access when I couldn't avoid it.
I will try to be clearer with some code, first this:
class InfoProcessor; // generate analytic data from info - non-copyable!
Is it movable?
In my case no. If it was it would be ok.
class Sequence { boost::synchronized_value<**InfoProcessor> m_infoproc;
public: explicit Sequence( SequenceInfo info ) : m_infoproc( info ) // process info into the constructor, ready to be used
Isn't the parameter InfoProcessor info?
No, it's SequenceInfo. m_infoproc "unflat" data from the basic info provided.
{
m_infoproc->do_something(); // can't avoid a lock? }
Sequence( const Sequence& ) = delete; Sequence& operator=( const Sequence& ) = delete;
Assuming I can't modify InfoProcessor, the only way I see how to do the call in the constructor without locking would be to encapsulate the member into a wrapper:
class Sequence { struct InfoProcWrapper { InfoProcWrapper( SequenceInfo info ) : infoproc( info ) { infoproc.do_something(); } // no locking
InfoProcessor infoproc; }; boost::synchronized_value<**InfoProcWrapper > m_infoproc;
public: explicit Sequence( SequenceInfo info ) : m_infoproc( info ) // now any access will need locking { // OK: m_infoproc->infoproc.do_**something() was already called without locking. }
Which would be fine if it wasn't so noisy and that's only for 1 synchronized member...
I agree, creating these wrappers is too noisy but an alternative. I think the factory is a better choice. Note that you have lambdas in C++11.
Yes I thought about lambdas afterward, but assuming InforProc is not movable nor copyable, the factory you are suggesting would have to create it dynamically to be able to pass it around. It's again a solution, not perfect for all cases but I guess that's ok in most cases.
class Sequence { struct InfoProcWrapper { InfoProcWrapper( SequenceInfo info ) : infoproc( info ) { infoproc.do_something(); } // no locking
InfoProcessor infoproc; }; struct DataCruncherWrapper { DataCruncherWrapper( SequenceInfo info ) : cruncher( info ) { cruncher.prepare( 42 ); } // no locking
DataCruncher cruncher; }; boost::synchronized_value<**InfoProcWrapper> m_infoproc; boost::synchronized_value<**DataCruncher> m_cruncher; // should NOT be synched with the same mutex
public: explicit Sequence( SequenceInfo info ) : m_infoproc( info ) // OK: setup with no locking : m_cruncher( info ) // OK: setup with no locking { m_cruncher->cruncher.**configure( m_infoproc->infoproc ); // how to do that without locking? }
Delaying the construction. This is similar to the problems raised for writing constexpr fuctions.
class Sequence { struct X { InfoProcWrapper m_infoproc; DataCruncher m_cruncher; X(SequenceInfo info) : sx_( info ) : m_cruncher( info )
{ m_cruncher->cruncher.**configure( m_infoproc->infoproc ); } };
struct SX {
boost::synchronized_value<**InfoProcWrapper> m_infoproc; boost::synchronized_value<**DataCruncher> m_cruncher; // should NOT be synched with the same mutex SX(X) {...}; }; SX sx_;
public: explicit Sequence( SequenceInfo info ) : sx_( X(info) )
{ }
Ok but that works only if InforProc and DataCruncher are at least movable. So, unperfect but works in a lot of cases.
Anyway, I might be wrong but it seems to me that any attempt to combine use of several synched members in the constructor will imply locking OR adding several layers of abstractions, which if the members' types have value-semantic, can be easy, but if it's not the case, it becomes quickly very noisy.
I agree. We surely need something that make this easier and less noisy. I would prefer if we continue to see other alternatives to go directly to providing a bypass function.
Yes. I am thinking maybe part of the solution would be to have a second optional argument in the synchronized_value<> constructor which would be lambda which would be called in the constructor body of the synchronized_value<>: explicit Sequence( SequenceInfo info ) : m_infoproc( info , []( InfoProcessor& infoproc ){ infoproc.do_something(); } ) // no locking - lamda called in synchronized_value<> constructor It don't solve the case where you want to use several members in the Sequence construuctor though. Is there any way in C++ to detect if we are in an object's construction? I think not. It would the opposite of the proposal for detecting when an exception have been thrown and stack-unwinding is occuring. I can't think of a better solution right now.
Note that hopefully the cost of the lock isn't important so far in my specific use case of today, I was just pointing the constructor cost because in the other code bases I have I might have more performance-related concerns and I might not be the only one.
I would help you if I can.
Thanks! Joel Lamotte