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

[Kieran Bingham]

Hi Laurent,

On 10/09/2019 11:08, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Tue, Sep 10, 2019 at 11:02:56AM +0100, Kieran Bingham wrote:
>>
>>> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>
>> Thanks, I'll update the commit message here as there is indeed no actual
>> race.
>>
>> But it does make me feel a lot better to hook up the dependencies first :-)

Is this rewrite acceptable to you ?


test: process: Connect signal before launching process

The procFinished event handler is registered after the process is
started. This doesn't actually create any race, as the finished signal
is emitted after a SIGCHLD is caught and handled through the
ProcessManager and processed by the event loop.

To make it clear that resources are connected before the process runs,
register the handler before the process is started.



>>>> 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
--
Kieran