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

[Zbigniew Kempczyński]

On Mon, Sep 28, 2020 at 06:29:20PM +0300, Petri Latvala wrote:
> 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?

Sure, shouldn't take much time. 

--
Zbigniew

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