[igt-dev] [PATCH i-g-t] lib/igt_core: Reinitialize print mutex in child process

[Petri Latvala]

On Mon, Sep 28, 2020 at 03:15:02PM +0200, Zbigniew Kempczyński wrote:
> IGT are prone to deadlock in igt_log() in following scenario:
> 
> 1. Parent process creates additional thread which for example
>    is doing endless loop
> 2. Thread sometimes is logging to console using igt_info().
>    This locks and unlocks print_mutex.
> 3. If in the meantime parent process will spawn it can be
>    spawned with print_mutex locked. If in the child process
>    it will do igt_info() it will deadlock.
> 
> We should reinitialize print_mutex in child process to avoid
> use inherited value.
> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> Cc: Petri Latvala <petri.latvala at intel.com>
> ---

Are you able to type up a unit test in lib/tests/ that reproduces this
deadlock?

Code LGTM, Arek, do you spot anything amiss with this?


-- 
Petri Latvala



>  lib/igt_core.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index cedd8168..722202ae 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -2271,6 +2271,8 @@ static void children_exit_handler(int sig)
>  		;
>  }
>  
> +static pthread_mutex_t print_mutex = PTHREAD_MUTEX_INITIALIZER;
> +
>  bool __igt_fork(void)
>  {
>  	internal_assert(!test_with_subtests || in_subtest,
> @@ -2300,6 +2302,7 @@ bool __igt_fork(void)
>  		igt_assert(0);
>  	case 0:
>  		test_child = true;
> +		pthread_mutex_init(&print_mutex, NULL);
>  		exit_handler_count = 0;
>  		reset_helper_process_list();
>  		oom_adjust_for_doom();
> @@ -2737,8 +2740,6 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
>  		"NONE"
>  	};
>  
> -	static pthread_mutex_t print_mutex = PTHREAD_MUTEX_INITIALIZER;
> -
>  	assert(format);
>  
>  #ifdef __GLIBC__
> @@ -2747,7 +2748,6 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
>  	program_name = command_str;
>  #endif
>  
> -
>  	if (igt_thread_is_main()) {
>  		thread_id = strdup("");
>  	} else {
> @@ -2823,6 +2823,8 @@ out:
>  	free(line);
>  }
>  
> +
> +
>  static const char *timeout_op;
>  static void __attribute__((noreturn)) igt_alarm_handler(int signal)
>  {
> -- 
> 2.26.0
>