[libcamera-devel,07/12] test: message: Destroy Object from correct thread context
diff mbox series

Message ID 20240121035948.4226-8-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Hardening against thread race conditions
Related show

Commit Message

Laurent Pinchart Jan. 21, 2024, 3:59 a.m. UTC
The MessageReceiver and RecursiveMessageReceiver used in the test are
destroyed from the main thread, which is invalid for a thread-bound
object bound to a different thread. Fix it by destroying them with
deleteLater().

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 test/message.cpp | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

Comments

Milan Zamazal Jan. 22, 2024, 8:03 p.m. UTC | #1
Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> The MessageReceiver and RecursiveMessageReceiver used in the test are
> destroyed from the main thread, which is invalid for a thread-bound
> object bound to a different thread. Fix it by destroying them with
> deleteLater().
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Milan Zamazal <mzamazal@redhat.com>

> ---
>  test/message.cpp | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/test/message.cpp b/test/message.cpp
> index a34e0f0b5e10..2f9f281c5101 100644
> --- a/test/message.cpp
> +++ b/test/message.cpp
> @@ -109,16 +109,19 @@ protected:
>  			return TestFail;
>  		}
>  
> -		MessageReceiver receiver;
> -		receiver.moveToThread(&thread_);
> +		MessageReceiver *receiver = new MessageReceiver();
> +		receiver->moveToThread(&thread_);
>  
>  		thread_.start();
>  
> -		receiver.postMessage(std::make_unique<Message>(Message::None));
> +		receiver->postMessage(std::make_unique<Message>(Message::None));
>  
>  		this_thread::sleep_for(chrono::milliseconds(100));
>  
> -		switch (receiver.status()) {
> +		MessageReceiver::Status status = receiver->status();
> +		receiver->deleteLater();
> +
> +		switch (status) {
>  		case MessageReceiver::NoMessage:
>  			cout << "No message received" << endl;
>  			return TestFail;
> @@ -135,8 +138,7 @@ protected:
>  		 * leaks. Two messages need to be posted to ensure we don't only
>  		 * test the simple case of a queue containing a single message.
>  		 */
> -		std::unique_ptr<RecursiveMessageReceiver> recursiveReceiver =
> -			std::make_unique<RecursiveMessageReceiver>();
> +		RecursiveMessageReceiver *recursiveReceiver = new RecursiveMessageReceiver();
>  		recursiveReceiver->moveToThread(&thread_);
>  
>  		recursiveReceiver->postMessage(std::make_unique<Message>(Message::None));
> @@ -144,7 +146,10 @@ protected:
>  
>  		this_thread::sleep_for(chrono::milliseconds(10));
>  
> -		if (!recursiveReceiver->success()) {
> +		bool success = recursiveReceiver->success();
> +		recursiveReceiver->deleteLater();
> +
> +		if (!success) {
>  			cout << "Recursive message delivery failed" << endl;
>  			return TestFail;
>  		}

Patch
diff mbox series

diff --git a/test/message.cpp b/test/message.cpp
index a34e0f0b5e10..2f9f281c5101 100644
--- a/test/message.cpp
+++ b/test/message.cpp
@@ -109,16 +109,19 @@  protected:
 			return TestFail;
 		}
 
-		MessageReceiver receiver;
-		receiver.moveToThread(&thread_);
+		MessageReceiver *receiver = new MessageReceiver();
+		receiver->moveToThread(&thread_);
 
 		thread_.start();
 
-		receiver.postMessage(std::make_unique<Message>(Message::None));
+		receiver->postMessage(std::make_unique<Message>(Message::None));
 
 		this_thread::sleep_for(chrono::milliseconds(100));
 
-		switch (receiver.status()) {
+		MessageReceiver::Status status = receiver->status();
+		receiver->deleteLater();
+
+		switch (status) {
 		case MessageReceiver::NoMessage:
 			cout << "No message received" << endl;
 			return TestFail;
@@ -135,8 +138,7 @@  protected:
 		 * leaks. Two messages need to be posted to ensure we don't only
 		 * test the simple case of a queue containing a single message.
 		 */
-		std::unique_ptr<RecursiveMessageReceiver> recursiveReceiver =
-			std::make_unique<RecursiveMessageReceiver>();
+		RecursiveMessageReceiver *recursiveReceiver = new RecursiveMessageReceiver();
 		recursiveReceiver->moveToThread(&thread_);
 
 		recursiveReceiver->postMessage(std::make_unique<Message>(Message::None));
@@ -144,7 +146,10 @@  protected:
 
 		this_thread::sleep_for(chrono::milliseconds(10));
 
-		if (!recursiveReceiver->success()) {
+		bool success = recursiveReceiver->success();
+		recursiveReceiver->deleteLater();
+
+		if (!success) {
 			cout << "Recursive message delivery failed" << endl;
 			return TestFail;
 		}