[libcamera-devel] [PATCH 1/3] test: process: Fix forking race

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tue, Sep 10, 2019 at 10:04:16AM +0100, Kieran Bingham wrote:
> The procFinished event handler is registered after the process is
> started, leading to the opportunity of a missed race.

That shouldn't be the case, as the procFinished signal is emitted from
the event loop. Still, connecting the signal before starting the process
is a good practice, so this patch is fine. You may want to mention in
the commit message that there's no actual race though.

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

> Register the handler before the process is launched.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  test/process/process_test.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp
> index d264555e545f..701f156b5053 100644
> --- a/test/process/process_test.cpp
> +++ b/test/process/process_test.cpp
> @@ -47,12 +47,13 @@ protected:
>  		int exitCode = 42;
>  		vector<std::string> args;
>  		args.push_back(to_string(exitCode));
> +		proc_.finished.connect(this, &ProcessTest::procFinished);
> +
>  		int ret = proc_.start("/proc/self/exe", args);
>  		if (ret) {
>  			cerr << "failed to start process" << endl;
>  			return TestFail;
>  		}
> -		proc_.finished.connect(this, &ProcessTest::procFinished);
>  
>  		timeout.start(100);
>  		while (timeout.isRunning())

-- 
Regards,

Laurent Pinchart