Re: monit restart bug/race condition (3.1 and current CVS version behavior)

[Martin Pala]

Jan-Henrik Haukeland wrote:

> Martin Pala <address@hidden> writes:
>
> [clever stuff]
>  
>
> > I tried it on simple test -  in this case monit without action mutex
> > is less affected then monit with action mutex in check_process(),
> > which failes every time. To solve the race condition, it will be
> > needed to implement probably per-process mutex before do_validate flag
> > test and cervlet check_process() calls. I did it too, but i found a
> > deadlock in that case => it needs more work (last 'development' patch
> > attached -
> >
> > probably there could be better way to do it).
> >    
>
> This looks okay!  The reason it does not work is that you will need to
> initialize the mutex first :-) When the process is created in
> p.y:createprocess() do a, pthread_mutex_init(&current->mutex, NULL);
> and likewise in gc.c:gc_process() do a
> pthread_mutex_destroy(&(*p)->mutex);
You got it - thank you :)

> And the validate.c:do_not_validate() and util.is_process_running()
> methods should (probably) use the same mutex,
I think it will be better to not use that mutex here because it is 
highly dependant on context. In addition if we put mutex to 
do_not_validate() i think it will end up in deadlock:

do_validate()
lock mutex
do_not_validate() -> would block on above locked mutex => deadlock
...

> but the main reason to
> use a mutex is that process's start/stop methods are synchronized
> (i.e. serialized) per process to avoid stuff like this:
>
>        (restart)
>        thread1:                thread2
>
>        process.stop            
>                                is(!process.running) -> it's not running
>                                 process.start
>        process.start
>
> With mutex
>
>        (restart)
>        thread1:                thread2
>
>        LOCK
>         process.stop            
>         process.start
>        END_LOCK
>                                LOCK
>                                 is(!process.running) -> it's running
>                                  process.start
>                                END_LOCK
>
>
>
> static int do_not_validate(Process_T p) {
>  
>  int rv= TRUE;        
>
>  ASSERT(p);
>
>  LOCK(p->mutex)
>        rv= (!p->do_validate  ||
>             check_skip(p)    ||
>             check_timeout(p) ||
>             check_checksum(p));
>  END_LOCK;
>
>  return rv;
>
> }
>
>