[libcamera-devel,08/12] test: signal-threads: Destroy Object from correct thread context
diff mbox series

Message ID 20240121035948.4226-9-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 SignalReceiver used in the test is destroyed from the main thread,
which is invalid for a thread-bound object bound to a different thread.
Fix it by destroying it with deleteLater().

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 test/signal-threads.cpp | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

Comments

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

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

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

> ---
>  test/signal-threads.cpp | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/test/signal-threads.cpp b/test/signal-threads.cpp
> index 8c550eb014d8..8c212b6f9ade 100644
> --- a/test/signal-threads.cpp
> +++ b/test/signal-threads.cpp
> @@ -59,15 +59,20 @@ private:
>  class SignalThreadsTest : public Test
>  {
>  protected:
> +	int init()
> +	{
> +		receiver_ = new SignalReceiver();
> +		signal_.connect(receiver_, &SignalReceiver::slot);
> +
> +		return TestPass;
> +	}
> +
>  	int run()
>  	{
> -		SignalReceiver receiver;
> -		signal_.connect(&receiver, &SignalReceiver::slot);
> -
>  		/* Test that a signal is received in the main thread. */
>  		signal_.emit(0);
>  
> -		switch (receiver.status()) {
> +		switch (receiver_->status()) {
>  		case SignalReceiver::NoSignal:
>  			cout << "No signal received for direct connection" << endl;
>  			return TestFail;
> @@ -83,8 +88,8 @@ protected:
>  		 * Move the object to a thread and verify that the signal is
>  		 * correctly delivered, with the correct data.
>  		 */
> -		receiver.reset();
> -		receiver.moveToThread(&thread_);
> +		receiver_->reset();
> +		receiver_->moveToThread(&thread_);
>  
>  		thread_.start();
>  
> @@ -92,7 +97,7 @@ protected:
>  
>  		this_thread::sleep_for(chrono::milliseconds(100));
>  
> -		switch (receiver.status()) {
> +		switch (receiver_->status()) {
>  		case SignalReceiver::NoSignal:
>  			cout << "No signal received for message connection" << endl;
>  			return TestFail;
> @@ -104,7 +109,7 @@ protected:
>  			break;
>  		}
>  
> -		if (receiver.value() != 42) {
> +		if (receiver_->value() != 42) {
>  			cout << "Signal received with incorrect value" << endl;
>  			return TestFail;
>  		}
> @@ -114,11 +119,13 @@ protected:
>  
>  	void cleanup()
>  	{
> +		receiver_->deleteLater();
>  		thread_.exit(0);
>  		thread_.wait();
>  	}
>  
>  private:
> +	SignalReceiver *receiver_;
>  	Thread thread_;
>  
>  	Signal<int> signal_;

Patch
diff mbox series

diff --git a/test/signal-threads.cpp b/test/signal-threads.cpp
index 8c550eb014d8..8c212b6f9ade 100644
--- a/test/signal-threads.cpp
+++ b/test/signal-threads.cpp
@@ -59,15 +59,20 @@  private:
 class SignalThreadsTest : public Test
 {
 protected:
+	int init()
+	{
+		receiver_ = new SignalReceiver();
+		signal_.connect(receiver_, &SignalReceiver::slot);
+
+		return TestPass;
+	}
+
 	int run()
 	{
-		SignalReceiver receiver;
-		signal_.connect(&receiver, &SignalReceiver::slot);
-
 		/* Test that a signal is received in the main thread. */
 		signal_.emit(0);
 
-		switch (receiver.status()) {
+		switch (receiver_->status()) {
 		case SignalReceiver::NoSignal:
 			cout << "No signal received for direct connection" << endl;
 			return TestFail;
@@ -83,8 +88,8 @@  protected:
 		 * Move the object to a thread and verify that the signal is
 		 * correctly delivered, with the correct data.
 		 */
-		receiver.reset();
-		receiver.moveToThread(&thread_);
+		receiver_->reset();
+		receiver_->moveToThread(&thread_);
 
 		thread_.start();
 
@@ -92,7 +97,7 @@  protected:
 
 		this_thread::sleep_for(chrono::milliseconds(100));
 
-		switch (receiver.status()) {
+		switch (receiver_->status()) {
 		case SignalReceiver::NoSignal:
 			cout << "No signal received for message connection" << endl;
 			return TestFail;
@@ -104,7 +109,7 @@  protected:
 			break;
 		}
 
-		if (receiver.value() != 42) {
+		if (receiver_->value() != 42) {
 			cout << "Signal received with incorrect value" << endl;
 			return TestFail;
 		}
@@ -114,11 +119,13 @@  protected:
 
 	void cleanup()
 	{
+		receiver_->deleteLater();
 		thread_.exit(0);
 		thread_.wait();
 	}
 
 private:
+	SignalReceiver *receiver_;
 	Thread thread_;
 
 	Signal<int> signal_;