I think I have found a issue while using sigslot::observer. (Reproducable in MSVC2015 and MSVC2019)
This is my example code:
struct A : sigslot::observer {
void my_slot() { ; }
};
sigslot::signal<> my_signal;
// Example 1
{
A a;
my_signal.connect(&A::my_slot, &a); // <-- Call's overload with observer
}
// Works as expected: my_slot is not called
my_signal();
// Example 2
A* aptr2 = new A;
A& aptr2derefed = *aptr2;
my_signal.connect(&A::my_slot, &aptr2derefed); // <-- Call's overload with observer
delete aptr2;
// Works as expected: my_slot is not called
my_signal();
// Example 3
A* aptr = new A;
my_signal.connect(&A::my_slot, aptr); // <-- Call's overload without observer
delete aptr;
// my_slot gets called even though aptr is already deleted
my_signal();
Example 1 and 2 work as I expect and my_slot does not get called in example 3 however a differnet overload
of connect gets called, where !trait::is_observer_v<Ptr> is checked in std::enable_if. This is an issue because
after aptr is deleted and my_signal gets emited my_slot gets called on a dangling pointer.
It could be a misuse case on my side but I think it might be an error in is_observer_v.
When you add a std::remove_reference_t to is_observer_v the issue seems to be fixed but I'm not sure if this causes any
unwanted side effects.
Hi. First of all thanks for this great library.
I think I have found a issue while using
sigslot::observer
. (Reproducable in MSVC2015 and MSVC2019)This is my example code:
Example 1 and 2 work as I expect and my_slot does not get called in example 3 however a differnet overload of
connect
gets called, where!trait::is_observer_v<Ptr>
is checked instd::enable_if
. This is an issue because afteraptr
is deleted andmy_signal
gets emitedmy_slot
gets called on a dangling pointer. It could be a misuse case on my side but I think it might be an error inis_observer_v
. When you add astd::remove_reference_t
tois_observer_v
the issue seems to be fixed but I'm not sure if this causes any unwanted side effects.This is my proposed fix for this issue: