0

I would like to create a wrapper class for boost::signals2 for modules (threads) that emits signals to slots. I.e. a module should gain typical simple signalling capabilities (e.g. a public connect(...) method) by inheriting from my Signal class. I would also like to hide the actual signal-slot implementation that is used.

A concrete slot inherits from a generic Slot base class which has a template parameter defining its signature. A slot is just a functor with the suitable signature.

This question is somewhat related to this question. Slots are stored as shared_ptr and lifetime management is required. I.e. the Signal class should hold a reference to the slot to keep it alive as long as the signal itself exits. Hence I cannot connect std::functions or similar. I have to connect shared_ptrs of the slot base class.

My current approach, without thread safety so far (MSVC 2010):

template<class FunSig>
class Slot;

template<class R>
class Slot<R()>
{
public:
    typedef R Ret_type;

public:
    virtual ~Slot() {}
    virtual Ret_type operator()() = 0;
};

template<class R, class A1>
class Slot<R(A1)>
{
public:
    typedef R Ret_type;
    typedef A1 Arg1_type;

public:
    virtual ~Slot() {}
    virtual Ret_type operator()(Arg1_type) = 0;
};

// and so forth for more arguments


/*
Signalling class.
This class is basically a wrapper for the boost::signals2 class with 
lifetime management for slots.
Slots are connected by a shared_ptr which gets stored
in a std::vector to ensure that a slot exists at least as long as the signal.
*/
template<class FunSig>
class Signal
{
public:
    typedef Slot<FunSig> Slot_type;
    typedef boost::signals2::signal<FunSig> BoostSignal;
    typedef typename BoostSignal::slot_type BoostSlot;

public:
    virtual ~Signal() {}

    void connectSlot(std::shared_ptr<Slot_type> slot_ptr);

protected:
    //void emitSignal( ... );
    //void disconnectAllSlots();

private:
    BoostSignal sig_;

    /// vector of shared_ptr to slots for lifetime management
    std::vector<std::shared_ptr<Slot_type> > slotsVec_;
};


template<class FunSig>
void Signal<FunSig>::connectSlot(std::shared_ptr<Slot_type> slot_ptr)
{
    sig_.connect(*slot_ptr); // version A: compiler error

    // OR

    sig_.connect(boost::ref(*slot_ptr)); // version B: warning, but compiles and runs


    // add slot pointer to vector of slots
    slotsVec_.push_back(slot_ptr);
}

This code (version A) does not compile. It breaks inside boosts slot_template.hpp and at the line marked in the connectSlot method:

error C2679: binary '=' : no operator found which takes a right-hand operand of type 'const Slot<FunSig>' (or there is no acceptable conversion)
1>          with
1>          [
1>              FunSig=void (const float &)

Interestingly this code compiles and runs if version B is used instead - i.e. a boost::ref is passed of the slot. Though there is a compiler warning "Function call with parameters that may be unsafe - this call relies on the caller to check that the passed values are correct." in boost's singals2 auto_buffer.hpp.

So what is the actual problem here and how to solve it? Why does this work with boost::ref and why does it not compile without it?

I am even unsure it the whole design idea is useful. The original idea was to hide the whole signalling/slot stuff in a superclass and focus on the signature (and include lifetime management).

An additional question regarding boost's signals2: the singals2 connect() method takes a reference to a slot. How is this handled internally. Does it use a reference of the connected slot or does it make a copy of the slot? This is important as my slots handle dynamically allocated memory.

Community
  • 1
  • 1
spinxz
  • 401
  • 3
  • 13
  • First, experiment with turning your `slot_type` into a `boost` slot type. Second, be careful with value semantics and a pure virtual interface. Third, given that your slot type is an interface that says "I have an op()", `std::function` seems better. Forth, an abstract interface with no virtual destructor being held in a smart pointer? – Yakk - Adam Nevraumont Apr 26 '13 at 12:53
  • @Yakk thanks. What do you mean by "be careful with value semantics..."? connectSlot() and boost's connect() both take their arguments as pointer/reference. – spinxz Apr 26 '13 at 13:32
  • But do they *store* the argument by value internally? – Yakk - Adam Nevraumont Apr 26 '13 at 13:49
  • I don't know if the slot is handled by value internally. But usually a slot is connected without using boost::ref, see [examples here](http://www.boost.org/doc/libs/1_51_0/doc/html/signals2/tutorial.html#id3207011). I do not understand why the code does not compile if boost::ref is not used. I guess it has to do with my generic slot class... Using BoostSlot in the connect method instead of my Slot_type is not an option because all slots all handled with shared_ptrs which are not convertible to share_ptr. – spinxz Apr 29 '13 at 09:38

2 Answers2

1

I assume you mean the warning C4996. This is a feature of Microsoft's c++ standard library implementation which warns you whenever a standard algorithm is compiled with potentially unsafe arguments, e.g. in this snippet, the caller is responsible for sufficient sizes of source and target:

int source[3] = { 1, 2, 3 };
int target[3];
std::copy(source, source + 3, target);

You should not duplicate the functionality already provided by Boost.Signals2: the library provides a sophisticated life-time management of the "slots" connected to a signal. Read through the docs, they foresee a very fine-granular control of this aspect. Also, you lose a very interesting feature - the thread safety of Boost.Signals2: You'd need to manage thread safe insertions and deletions into and from the slotVec_ yourself which is a non-trivial undertaking...

I am working on a similar abstraction library called vex myself, have a look at abstract_multicast_delegate and abstract_signal in vex/functional/contracts. The implementation based on signals2::signal can be found in vex/functional/implementation/bs2_signal.h. This is however still a playground, I am experimenting with alternative implementations too...

EDIT: Sorry, I don't know how to point to the tip of a hg repository at codeplex. I had to remove the links since a lot has changed since yesterday...

Paul Michalik
  • 4,331
  • 16
  • 18
  • Thanks for this answer. I guess regarding lifetime management you are referring to slot [tracking](http://www.boost.org/doc/libs/1_51_0/doc/html/boost/signals2/slot.html). As far as I understood slot.track(), it disconnects a slot if it expires. In contrast, I want to keep it alive as long as the signal exists. – spinxz Apr 29 '13 at 08:51
  • In your bs2_signal.h you are also connecting an any_delegate without using boost::ref ("p_signal.connect(any_delegate(p_delegate)"). In my case this does not compile. Maybe this works because you are using boost::signals2 ExtendedSlotFunction template parameter for the signal?! – spinxz Apr 29 '13 at 16:31
  • 1
    The requirements for the type of parameter passed to `signal` are not that strict: it must be either implicitely convertible to `slot_type` or be the `slot_type` itself. The `std::ref`ed wrapper compiles because `reference_wrapper` (result of `ref`) is callable. As I see it, a more serious problem is that you take a reference to the `shared_ptr` instance which lives only during the function call... You can pass a lambda or result of `std::bind` which captures a standard conforming smart pointer to the carrier object. This will keep your object alive as long as the signal exists. – Paul Michalik Apr 30 '13 at 18:08
  • With regard to the FunctionType question: No, that's not the reason. any_delegate is a callable "smart pointer" which conforms to the requirements signal2 puts on the function type. It already is a polymorphic wrapper around a callable, so no need for another one... – Paul Michalik Apr 30 '13 at 19:04
0

This question has been answered in a different context here.

Actually std::ref or boost::ref have to be used because boost::signals2 connect method copies its arguments. But the class Slot is not copyable as it is an abstract class. Hence using boost::ref is the recommended solution because it makes the slot copyable.

Community
  • 1
  • 1
spinxz
  • 401
  • 3
  • 13