[libcamera-devel] [PATCH v2] test: gstreamer: Disable gstreamer registry forks

[Laurent Pinchart]

Hi Vedant,

Thank you for the patch.

Looks like you've found an answer to the question you asked on my review
of v1 :-)

On Sat, Aug 21, 2021 at 08:11:45PM +0530, Vedant Paranjape wrote:
> ASan needs to be loaded first before gstreamer is loaded. This was not
> possible, so verify_asan_link_order was disabled. Better way to tackle
> this issue was disabling forks on the gstreamer side.
> 
> verify_asan_link_order=0 disables the check on ASan side which checks if
> ASan was loaded before any other DLLs. Since, gstreamer spawns a child

s/DLLs/shared objects/ as we're on Linux, no Windows :-)

> helper process while building the registry, we needed to disable this
> check. But with gst_registry_fork_set_enabled() it is possible to
> disable spawning this child helper process, so this ensures that ASan is
> loaded before any other DLL is loaded.
> 
> Signed-off-by: Vedant Paranjape <vedantparanjape160201 at gmail.com>
> ---
>  test/gstreamer/gstreamer_single_stream_test.cpp | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp
> index 349dcfa4..80e652eb 100644
> --- a/test/gstreamer/gstreamer_single_stream_test.cpp
> +++ b/test/gstreamer/gstreamer_single_stream_test.cpp
> @@ -41,18 +41,13 @@ protected:

The comment starts with "GStreamer by spawns a process ...". Now that we
know this can be changed with gst_registry_fork_set_enabled(), I think
this should be changed to "GStreamer by default spawns a process ...".

>  		 * GStreamer is most likely not, this will cause the ASan link
>  		 * order check to fail when gst-plugin-scanner dlopen()s the
>  		 * plugin as many libraries will have already been loaded by
> -		 * then. Work around this issue by disabling the link order
> -		 * check. This will only affect child processes, as ASan is
> -		 * already loaded for this process by the time this code is
> -		 * executed, and should thus hopefully be safe.
> -		 *
> -		 * This option is not available in gcc older than 8, the only
> -		 * option in that case is to skip the test.
> +		 * then. Work around this issue by disabling spawning of a
> +		 * child helper process when rebuilding the registry. This will

How about "when scanning the build directory for plugins" instead of
"when rebuilding the registry", as the problems occurs when
gst_registry_scan_path() is called ?

> +		 * only affect child processes, as ASan is already loaded for
> +		 * this process by the time this code is executed, and should
> +		 * thus hopefully be safe.

I think the last sentence can be dropped. I wrote it to document that
verify_asan_link_order=0 didn't disable the check for the current
process, which isn't a concern anymore as we don't disable the check
now.

If you're fine with those changes, I can update the patch when pushing,
there's no need to send a v3.

>  		 */
> -#if defined(__SANITIZE_ADDRESS__) && !defined(__clang__) && __GNUC__ < 8
> -		return TestSkip;
> -#endif
> -		setenv("ASAN_OPTIONS", "verify_asan_link_order=0", 1);
> +		gst_registry_fork_set_enabled(false);
>  
>  		/* Initialize GStreamer */
>  		g_autoptr(GError) errInit = NULL;

-- 
Regards,

Laurent Pinchart