Signal to signal connection support

[SandroWissmann]

In Qt you can connect signals to signals to forward a signal.

To have something like this:

signal 1 -> signal 2 -> slot

This is sometime really helpful if you need to promote a signal via several classes.

If I see the docs correctly that is currently not supported we would need to do make a slot which emits a signal:

signal 1 -> slot (emit signal 2 on call) -> signal 2 -> slot

Just wonder if you plan to add this. It could save a bit of boilerplate.

Beside that awesome library. I usually use qt but now I look for only signal slots in a project to avoid the heaviness of add the whole qt library.

[palacaze]

Hi Sandro

I actually implemented such feature in another branch already. You can find said commit here, would that work for you?

[SandroWissmann]

Hi thanks for quick reply.

I think thats exactly what im looking for.

I will grab the branch and try it out in my project. It could take a while but I can give you feedback.

[SandroWissmann]

One maybe stupid question.

Objects containing sigslot::signal as a member cannot be copied but moving should be safe right?

[palacaze]

Yes they can.

[SandroWissmann]

copied or moved? because copy somehow gives me compiler error that its implicitly deleted because of sigslot::signal (I guess similar qt were QObjects cannot be copied aswell)

[palacaze]

Can be moved. Copying signals makes no sense semantically speaking.

[SandroWissmann]

I tried it out so far and under linux no issue to compile the header.

The strange thing is if I compile it under windows with mingw.

I get this compile error:

Which is stupid because you include mutex on top.

Beside that I compile with c++17 and rest of the project compiles just fine.

MinGW Version:

[SandroWissmann]

OK forget about this. It is a problem in the mingw setup: https://stackoverflow.com/questions/14191566/c-mutex-in-namespace-std-does-not-name-a-type

[SandroWissmann]

I have trouble with connecting lambdas which need to pass an argument.

I have in a class Button a signal left_clicked.

I connect to it like this:

    for(int i = 0; i < m_buttons.size(); ++i)
    {
        sigslot::connect(
            m_buttons[i].left_clicked,
            [&] {
                on_entry_selected(i);
            });
    }

The problem is that on_entry_selected i is always 0 instead of 1, 2 ,3 etc. Is the capturing wrong here maybe?

edit: I think i solved the issue because of capture by refeference. If captured by value it works:

    for(int i = 0; i < m_buttons.size(); ++i)
    {
        sigslot::connect(
            m_buttons[i].left_clicked,
            [=] {
                on_entry_selected(i);
            });
    }

[palacaze]

Yeah you should capture i by value.

[SandroWissmann]

So far i'm very happy with this library.

But now I run into a situation which causes a very strange deadlock in signal.hpp

Somehow the program gets stuck here:

    void disconnect_all() {
        boost::nowide::cout << "disconnect_all before lock;\n";
        std::unique_lock<Lockable> _{m_mutex};
        boost::nowide::cout << "disconnect_all before m_connections.clear(); size: "<< m_connections.size() << '\n';  
        m_connections.clear();    // We get stuck here
        boost::nowide::cout << "disconnect_all finished\n";
    }

No idea why it should lock on a vector::clear operation but that''s what happens if I follow the logs.

I extracted the pieces which lead to this problem and created an minimal example were it can be reproduced here: https://github.com/SandroWissmann/SigslotDebug

Just look in main.cpp there is the signal slot flow which leads to this.

You can also run the example with:

mkdir build
cd build
./SigslotDebug

The path which leads to the deadlock in the example

int main()
{
    // ...
    windowManager.handle_input();

    // WindowMenuMain: Calls handle_input
    // Button: Calls handle_input
    // Button: Emits signal left_clicked
    // WindowMenuMain: Invokes slot on_button_left_clicked
    // WindowMenuMain: Emits signal change_window_requested
    // WindowManager: Invokes slot on_change_window_requested
    // WindowManager: Destroy WindowMenuMain         // we get stuck here
    // WindowManager: Create WindowFamilySelection
    std::cout << "int main() windowManager.windowManager.handle_input\n";
    windowManager.handle_input();  // we get stuck here

    std::cout << "int main() End\n";
}

Here the second handle_input is never executed until the end. Instead it gets stuck here:

    ~WindowMenuMain() override
    {
        std::cout << "~WindowMenuMain()\n";
        this->disconnect_all();   //  We get stuck here 
        std::cout << "~WindowMenuMain() this->disconnect_all\n";
    }

Which then leads to deadlock in line m_connections.clear(); in signal.hpp.

Currently i'm completely clueless if I misuse the library and caused this or if this is a bug.

I suspect it has something to do with sigslot::observer and calling this->disconnect_all() in the Destructor. From your docs I thought I could just always derrive only in the class which has a slot from sigslot::observer and then call this->disconnect_all()

[SandroWissmann]

Let me know if you need more information or something is unclear.

[palacaze]

I observe the deadlock, thanks for the reproducer.

The code in this branch is very much a proof of concept and what happens is that disconnect_all waits for all the currently invoked slots before returning. You piece of code gets stuck as a result from calling disconnect_all from within a slot, which cannot work right now in this branch as per my previous explanation.

The reason for this work is to offer a safe way of ensuring that a slot is not being invoked while destroying it, which has the unfortunate side effect of breaking your code.

Now I am not a fan of observer base classes, I added the feature because it was requested and felt reasonable at the time, but there are many gotchas associated with it. I think your design would improve if the manager owner the "change_window_requested" signal and if your widgets held a reference to the manager.

[SandroWissmann]

Ok I get it.

I was also thinking about adding doing the reference thing from WindowManager like you suggest. This whole signal emit from the base window class was probably a bad a idea.

Then what about Button which has a signal which is connected to WindowMenuMain. If WindowMenuMain gets destroyed do I still need to disconnect the signal slot connection from Button to WindowMenuMain ? Or do I not need to worry about it? Because right now ẀindowMenuMainis also derived frompublic sigslot::observer` because I thought you need to clean up the connection with the button.

[SandroWissmann]

Let me give an more abstract example to my question.

I will often have this constellation:

class A {
    signal clicked();
}

class B {

    B()
    {
        //Connect signal 
        sigslot::connect(
        a.clicked,
        &B::on_clicked,
        this);
    }
    //Slot
    void on_clicked();

    // class A as member
    A a;
}

So class B has class A as a member. signal from class B gets connected to slot of class A.

Now when class B gets destructed also class A gets destructed.

Now what about the signal slot connection. Do I have to manually disconnect it in the destructor of B? Im familiar with Qt and there I would disconnect anything. But I think there it is handled under the hood...

Just want to understand whats the right approach here...

[SandroWissmann]

Can you please answer the last question?

[palacaze]

In this example all the slots will be auto-disconnected upon destruction of the A object. B encapsulates A, so its lifetime is a strict superset of the lifetime of B, to A gets destroyed before B goes out of scope, the Signal will never hold dangling pointers to A.