[libcamera-devel] [PATCH 08/19] test: signal: Add additional disconnection tests for Object

[Jacopo Mondi]

Hi Laurent,

On Mon, Jan 20, 2020 at 02:24:26AM +0200, Laurent Pinchart wrote:
> Add two tests that exercise the Signal::disconnect(Object *) and
> Signal::disconnect() methods, to verify that they correctly remove the
> signal from the connected object's list of signals. This triggers an
> issue that was detected through manual code inspection, and is expected
> to crash or at least generate valgrind warnings.
>

I can confirm the following is gone in the next commits

==25012== Invalid read of size 8
==25012==    at 0x112E8D: std::__cxx11::list<libcamera::BoundMethodBase*, std::allocator<libcamera::BoundMethodBase*> >::begin() (stl_list.h:942)
==25012==    by 0x4A3253C: void libcamera::SignalBase::disconnect<libcamera::Object>(libcamera::Object*) (signal.h:25)
==25012==    by 0x4A31A0B: libcamera::Object::~Object() (object.cpp:80)
==25012==    by 0x11AA87: SlotObject::~SlotObject() (signal.cpp:31)
==25012==    by 0x11AAAB: SlotObject::~SlotObject() (signal.cpp:31)
==25012==    by 0x112536: SignalTest::run() (signal.cpp:233)
==25012==    by 0x11B41D: Test::execute() (test.cpp:36)
==25012==    by 0x1113E6: main (signal.cpp:317)
==25012==  Address 0x50d3690 is 0 bytes inside a block of size 24 free'd
==25012==    at 0x4839EAB: operator delete(void*) (vg_replace_malloc.c:586)
==25012==    by 0x11250E: SignalTest::run() (signal.cpp:232)
==25012==    by 0x11B41D: Test::execute() (test.cpp:36)
==25012==    by 0x1113E6: main (signal.cpp:317)
==25012==  Block was alloc'd at
==25012==    at 0x4838DEF: operator new(unsigned long) (vg_replace_malloc.c:344)
==25012==    by 0x11240D: SignalTest::run() (signal.cpp:228)
==25012==    by 0x11B41D: Test::execute() (test.cpp:36)
==25012==    by 0x1113E6: main (signal.cpp:317)

> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

> ---
>  test/signal.cpp | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/test/signal.cpp b/test/signal.cpp
> index 0054ed5a380d..f83ceb05f091 100644
> --- a/test/signal.cpp
> +++ b/test/signal.cpp
> @@ -220,6 +220,30 @@ protected:
>  		delete dynamicSignal;
>  		delete slotObject;
>
> +		/*
> +		 * Test that signal manual disconnection from Object removes
> +		 * the signal for the object. This shall not generate any
> +		 * valgrind warning.
> +		 */
> +		dynamicSignal = new Signal<>();
> +		slotObject = new SlotObject();
> +		dynamicSignal->connect(slotObject, &SlotObject::slot);
> +		dynamicSignal->disconnect(slotObject);
> +		delete dynamicSignal;
> +		delete slotObject;
> +
> +		/*
> +		 * Test that signal manual disconnection from all slots removes
> +		 * the signal for the object. This shall not generate any
> +		 * valgrind warning.
> +		 */
> +		dynamicSignal = new Signal<>();
> +		slotObject = new SlotObject();
> +		dynamicSignal->connect(slotObject, &SlotObject::slot);
> +		dynamicSignal->disconnect();
> +		delete dynamicSignal;
> +		delete slotObject;
> +
>  		/* Exercise the Object slot code paths. */
>  		slotObject = new SlotObject();
>  		signalVoid_.connect(slotObject, &SlotObject::slot);
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20200120/d06fe4bd/attachment.sig>