[libcamera-devel] [PATCH 2/2] test: camera: Camera reconfiguration and fd-leak test

[Laurent Pinchart]

Hi Kieran,

On Tue, Aug 17, 2021 at 02:57:24PM +0100, Kieran Bingham wrote:
> On 17/08/2021 14:04, Laurent Pinchart wrote:
> > Hi Umang,
> > >> +	pid_t findProxyPid()
> >> +	{
> >> +		string proxyPid;
> >> +		string proxyName(kIpaProxyName_);
> >> +		DIR *dir;
> >> +		struct dirent *ptr;
> >> +
> >> +		dir = opendir("/proc");
> >> +		while((ptr = readdir(dir)) != nullptr) {
> >> +			if(ptr->d_type != DT_DIR)
> >> +				continue;
> >> +
> >> +			string pname("/proc/" + string(ptr->d_name) + "/comm");
> >> +			if(File::exists(pname.c_str())) {
> > 
> > Missing space.
> > 
> > I may have missed it, but I haven't seen an answer to my review of the
> > previous version that proposed checking the number of open file
> > descriptors in the VIMC IPA module itself. That would test for fd leaks
> > in all tests using vimc, and wouldn't require finding child processes.
> 
> 
> I presume this would just fire an ASSERT() if it was detected. (Unless
> you have another way to report the failure up to the test?)
> 
> I'm a bit weary that this would be hard to track down when it fires as
> the failure would occur in a process which we don't (easily) get logs
> from. The failure condition might get a bit 'lost'.
> 
> The assert would cause the IPA to 'crash', and the tests would fail -
> but they would look like they were failing because the IPA had
> 'disappeared' and may not be obvious to diagnose..
> 
> 
> I wonder if we need to tackle how we log from isolated processes already
> ... :-S
> 
> 
> I guess the alternative is to have some extended interface callbacks
> from the IPA to allow it to message the VIMC pipeline handler to report
> something ... and ... cause a failure and shutdown on the pipeline side ?

We do have a tracing mechanism already, with a FIFO used to pass
information from the IPA to tests. It could be leveraged, possibly just
using the existing infrastructure to start with (it's limited to passing
32-bit operation codes for tracing), extending it later with support for
more data.

-- 
Regards,

Laurent Pinchart