3

I am using boost::signals2 under Red Hat Enterprise Linux 5.3.

My signal creates an object copy and sends it's pointer to subscribers. This was implemented for thread safety to prevent the worker thread from updating a string property on the object at the same time it is being read ( perhaps I should revisit the use of locks? ).

Anyway, my concern is with multiple subscribers that dereference the pointer to the copied object on their own thread. How can I control object lifetime? How can I know all subscribers are done with the object and it is safe to delete the object?

typedef boost::signals2::signal< void ( Parameter* ) > signalParameterChanged_t;
signalParameterChanged_t    m_signalParameterChanged;

// Worker Thread - Raises the signal
void Parameter::raiseParameterChangedSignal()
{
      Parameter* pParameterDeepCopied = new Parameter(*this);
      m_signalParameterChanged(pParameterDeepCopied);
}
// Read-Only Subscriber Thread(s) - GUI (and Event Logging thread ) handles signal
void ClientGui::onDeviceParameterChangedHandler( Parameter* pParameter)
{
      cout << pParameter->toString() << endl;
      delete pParameter;  // **** This only works for a single subscriber !!!
}

Thanks in advance for any tips or direction,

-Ed

Ed of the Mountain
  • 4,137
  • 3
  • 37
  • 49
  • Can you clarify how different threads interact with the signal? – Emile Cormier Jan 13 '10 at 04:14
  • It would also help if you list the member variables of Parameter. Use fake names if you have to. – Emile Cormier Jan 13 '10 at 04:24
  • I was passing-by-copy for thread-safety reasons. GUI is a read-only consumer of Parameter objects. GUI could not read a std::string property while worker was writing. An exception was thrown without copying Parameter. With a single receiver, having the GUI delete the Parameter copy works fine. Object lifetime became an issue when passing a Parameter* to multiple reader threads; the GUI, and an Event Logging Thread. Pass-by-value or shared_ptr both solve this problem. Parameter contains an internal stl collection of parameter Rule objects and mixture 6 POD and std::strings properties. – Ed of the Mountain Jan 14 '10 at 14:53
  • Parameter Member Variables: const string description, DeviceHealthStatusFlags deviceHealthStatusFlags,bool enabled, long lastQueryResult, const string name, ParameterDataType parameterDataType, DeviceHealthStatusFlags paramStatus, Device * parent, ruleCollection_t & ruleCollection () double scalar,SnmpObjectIdentifier * snmpObject, string typeName, const string units, const boost::any & value () bool visible () – Ed of the Mountain Jan 14 '10 at 15:01
  • Thanks for the extra info. If you decide to pass by shared_ptr, and any of your consumer threads can modify Parameter, then you will have a race condition if Parameter doesn't synchronize concurrent access. Consider making Parameter inherit from a read-only interface class (IReadOnlyParameter) with all const pure virtual methods and a virtual destructor. Then send a shared_ptr to your subscribers instead. – Emile Cormier Jan 14 '10 at 21:21
  • I like the I idea of using IReadOnlyMyClassName interfaces. Thank you for all your thoughtful replies! Good idea. – Ed of the Mountain Jan 19 '10 at 16:48

1 Answers1

2

If you really have to pass Parameter by pointer to your subscribers, then you should use boost::shared_ptr:

typedef boost::shared_ptr<Parameter> SharedParameterPtr;
typedef boost::signals2::signal< void ( SharedParameterPtr ) > signalParameterChanged_t;
signalParameterChanged_t    m_signalParameterChanged;

// The signal source
void Parameter::raiseParameterChangedSignal()
{
      SharedParameterPtr pParameterDeepCopied = new Parameter(*this);
      m_signalParameterChanged(pParameterDeepCopied);
}
// The subscriber's handler
void ClientGui::onDeviceParameterChangedHandler( SharedParameterPtr pParameter)
{
      cout << pParameter->toString() << endl;
}

The shared parameter object sent to your subscribers will be automatically deleted when its reference count becomes zero (i.e. it goes out of scope in all the handlers).

Is Parameter really so heavyweight that you need to send it to your subscribers via pointer?

EDIT:

Please note that using shared_ptr takes care of lifetime management, but will not relieve you of the responsibility to make concurrent reads/writes to/from the shared parameter object thread-safe. You may well want to pass-by-copy to your subscribers for thread-safety reasons alone. In your question, it's not clear enough to me what goes on thread-wise, so I can't give you more specific recommendations.

Is the thread calling raiseParameterChangedSignal() the same as your GUI thread? Some GUI toolkits don't allow concurrent use of their API by multiple threads.

Emile Cormier
  • 26,080
  • 13
  • 87
  • 116
  • Hi Emile, Thank you very much for your informative reply and clear code samples. The boost::shared_ptr is a solution. In my test case deletion occurs as you described. You made me rethink. I would *prefer* to pass-by-value rather than pointers. Yet when using pass-by-value I am disturbed by all the copy constructor activity I see in my test case. My debug prints show the signal resulted in 6 unexpected calls to the copy constructor. After the slot handler, 6 calls to the destructor. Do you have any insight into what all the activity is? Thank you very much for your assistance! -Ed – Ed of the Mountain Jan 12 '10 at 14:35
  • I can only account for two copy constructions in your own code: 1) Passing your parameter object by copy to signalParameterChanged_t::operator(), 2) signal2 library passing the parameter by copy to the handler. The rest must be from the internal workings of the signals library. I'm not going to attempt to parse the metaprogramming wizardry going on inside signals2, to see where the other copies take place. ;) Try to recompile with optimization turned on (say, -O2) and I bet some of the copy constructions will be optimized away. – Emile Cormier Jan 13 '10 at 02:18
  • If your Parameter class only has a few built-in types (like int, double, etc.) for member variables, then I say just go ahead and pass Parameter by copy. The nanoseconds spent making 6 copies will be negligible compared to the micro/milliseconds your GUI toolkit will spend rendering scalable fonts on the screen. – Emile Cormier Jan 13 '10 at 02:35
  • 1
    To add to my previous comment, you should really learn about premature optimization if you haven't already: http://stackoverflow.com/questions/385506/when-is-optimisation-premature – Emile Cormier Jan 13 '10 at 03:55
  • If your signal dispatching code ends up really being speed critical (determined by banchmarking/profiling), then there's another consideration: Depending on how clever your compiler's heap management is, dynamically allocating / deleting a new Parameter object can be orders of magnitude slower than creating a copy on the stack. See http://stackoverflow.com/questions/161053/c-which-is-faster-stack-allocation-or-heap-allocation – Emile Cormier Jan 13 '10 at 04:03
  • Emile: I've learned more about how boost::signals2 works. I thought from Qt experience w/signals/slots that the signals dispatched to multiple threads--thus the combiner could provide the hook point to delete if you wanted to control the moment of deletion at the end of all slot runs. It turns out that the thread support in signals2 is fairly trivial, and deleting in a combiner is no different from just deleting after the signal call. Thus I've removed my answer to this question, since the only relevant question the OP could be asking requires a shared_ptr. – HostileFork says dont trust SE Jan 13 '10 at 04:31
  • ...but having said the above: There's nothing wrong with enforcing a contract in which there is a deletion that happens on the signaling thread after all slots have run. Ideally you'd pair up that decision with passing a reference instead of a pointer, but even that cue won't stop people from saving a reference. The important thing is the contract--and you have the right to declare a contract in which you control the lifetime of an object you send to a caller. (Those who disagree should be using Java, not C++.) – HostileFork says dont trust SE Jan 13 '10 at 04:38
  • @Hostile: I agree with your last comment when it comes to references. I think it's fairly obvious to anyone that it's the caller who manages the lifetime of the object passed by reference, and not the callee. If the callee needs to store the object for future use, then it will copy-assign it. But when it comes to raw pointers, things are not so clear and you have to rely on documentation. Do I delete the object when I'm done? Will the object persist after the function call? – Emile Cormier Jan 13 '10 at 07:59
  • ... But with a shared_ptr, things are self-documenting. No need to worry about lifetime management: the object will delete itself automatically. If the callee needs a copy for future use, it can make one very cheaply (copying the shared_ptr merely increments the reference count). Why make your like more miserable with raw pointers when shared_ptr makes things so much easier? – Emile Cormier Jan 13 '10 at 08:11
  • ...Of course, smart pointers can be overused. Here's a passage from C++ Coding Standards: "Raw pointers are fine in code where the pointed-to-object is visible to only a restricted quantity of code (e.g., purely internal to a class, such as a Tree class's internal node navigation pointers)." I don't think the OP's case qualifies for such an exemption. All this might be moot anyways because the OP might be better off passing by copy for thread safety reasons. – Emile Cormier Jan 13 '10 at 08:16
  • Pass-by-value or shared_ptr is a better design. I will try pass-by-value and optimize only if needed. The Parameter class has a number of POD attributes, std::string, and a small internal stl collection of rule objects that must be copied. If performance warrants, ( thanks for the link to premature optimization ), I can make a lighter-weight version or even a structure just to pass status updates to the GUI. The stack versus heap performance difference is eye-opening. Thank you very much for taking the time to educate and make me think about this. – Ed of the Mountain Jan 14 '10 at 14:14
  • With -O3 optimizations I still saw 6 additional unexpected copy constructors and destructors. I am sure it is in boost signals2. However I have never found stepping into templatized code very productive. I expect the stack versus heap performance difference more than makes up for the added copy constructors. – Ed of the Mountain Jan 14 '10 at 14:28
  • Not a problem, Ed. I myself have already used boost signals to implement the observer pattern in a multi-threaded fashion with a GUI and logger, so I already know about the pitfalls. – Emile Cormier Jan 14 '10 at 21:40
  • Knowing that your Parameter has an STL container object, there may not be so much of a performance difference in stack vs heap allocation after all. Your embedded container object will use dynamic allocation to deep-copy itself, even if you pass your Parameter object by value. – Emile Cormier Jan 14 '10 at 21:41
  • If you are satisfied with the answer, you should accept it (click on checkmark) so that others know your problem has been resolved. (Or don't, it's up to you :) ). – Emile Cormier Jan 15 '10 at 00:42