[libcamera-devel] [PATCH] test: threads: Fix memory leak

[Sebastian Fricke]

Hey Laurent,

Thank you for the patch.

On 12.04.2021 00:36, Laurent Pinchart wrote:
>The last instance of Thread allocated in the test is never deleted, not

s/not/nor/ ?

>are other instances deleted in error paths. Use a std::unique_ptr<> to
>ensure deletion.

I tested this fix and valgrind does not report any leaked memory from
the libcamera tests. But just out of curiosity do you get a similar
report in valgrind:
```
basti at nanopct4:~/libcamera$ valgrind --leak-check=full --show-leak-kinds=all ninja -C build test
...
libcamera 0.0.0

   Configuration
     Enabled pipelines     : ipu3
                             raspberrypi
                             rkisp1
                             simple
                             uvcvideo
                             vimc
     Android support       : False
     GStreamer support     : False
     V4L2 emulation support: False
     cam application       : True
     qcam application      : False
     Unit tests            : True

Option werror is: true [default: true]
...
==6151== LEAK SUMMARY:
==6151==    definitely lost: 296 bytes in 1 blocks
==6151==    indirectly lost: 576 bytes in 2 blocks
==6151==      possibly lost: 0 bytes in 0 blocks
==6151==    still reachable: 2,725,231 bytes in 15,220 blocks
==6151==         suppressed: 0 bytes in 0 blocks
```

Those are all related to our build tool ninja.

>
>Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Tested-by: Sebastian Fricke <sebastian.fricke at posteo.net>

Greetings,
Sebastian

>---
> test/threads.cpp | 19 ++++++++-----------
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
>diff --git a/test/threads.cpp b/test/threads.cpp
>index b4b8d913cd2b..e0c50dc90a65 100644
>--- a/test/threads.cpp
>+++ b/test/threads.cpp
>@@ -7,6 +7,7 @@
>
> #include <chrono>
> #include <iostream>
>+#include <memory>
> #include <thread>
>
> #include "libcamera/internal/thread.h"
>@@ -45,23 +46,23 @@ protected:
> 	int run()
> 	{
> 		/* Test Thread() retrieval for the main thread. */
>-		Thread *thread = Thread::current();
>-		if (!thread) {
>+		Thread *mainThread = Thread::current();
>+		if (!mainThread) {
> 			cout << "Thread::current() failed to main thread"
> 			     << endl;
> 			return TestFail;
> 		}
>
>-		if (!thread->isRunning()) {
>+		if (!mainThread->isRunning()) {
> 			cout << "Main thread is not running" << endl;
> 			return TestFail;
> 		}
>
> 		/* Test starting the main thread, the test shall not crash. */
>-		thread->start();
>+		mainThread->start();
>
> 		/* Test the running state of a custom thread. */
>-		thread = new Thread();
>+		std::unique_ptr<Thread> thread = std::make_unique<Thread>();
> 		thread->start();
>
> 		if (!thread->isRunning()) {
>@@ -79,10 +80,8 @@ protected:
> 			return TestFail;
> 		}
>
>-		delete thread;
>-
> 		/* Test waiting for completion with a timeout. */
>-		thread = new DelayThread(chrono::milliseconds(500));
>+		thread = std::make_unique<DelayThread>(chrono::milliseconds(500));
> 		thread->start();
> 		thread->exit(0);
>
>@@ -100,10 +99,8 @@ protected:
> 			return TestFail;
> 		}
>
>-		delete thread;
>-
> 		/* Test waiting on a thread that isn't running. */
>-		thread = new Thread();
>+		thread = std::make_unique<Thread>();
>
> 		timeout = !thread->wait();
> 		if (timeout) {
>-- 
>Regards,
>
>Laurent Pinchart
>
>_______________________________________________
>libcamera-devel mailing list
>libcamera-devel at lists.libcamera.org
>https://lists.libcamera.org/listinfo/libcamera-devel