[PATCH] Fix a pthreadpool race

[Volker Lendecke]

On Fri, Dec 15, 2017 at 12:17:07PM +0100, Ralph Wuerthner wrote:
> Hi Volker!
> 
> On 07.12.2017 07:59, Volker Lendecke via samba-technical wrote:
> > Attached patch has the very nice explanation in the commit message. It
> > also removes the necessity for the preliminary patch. Runs in a
> > private autobuild now. I'll push once this survived and nobody objects
> > in the meantime.
> 
> Maybe I'm a little bit late, but please take a look at my attached patch
> set. It fixes the job-starvation-after-fork race without introducing a new
> mutex. By moving the check on pool->prefork_cond in front of the
> pthread_cond_timedwait() call on pool->condvar the thread will continue with
> processing jobs. Threads will only be killed if the job queue is empty.
> 
> I addition I wrote a new test case to test this scenario with multiple
> threads running in parallel and doing a fork with jobs sitting on the job
> queue and doing a 2nd fork when only idle threads are around. One todo for
> this test case would be to add a check that all idle threads are gone after
> the 2nd fork. Unfortunately I didn't found a portable solution without
> checking /proc/<pid>/task or running a ps command. Maybe somebody else on
> this list has a clever idea.
> 
> The patch set applies on master.

That is of course a lot more elegant. Looking through the code I have
one concern: The threads are kept busy throughout the fork, which is
good. I would like to see a good argument why the while(num_idle!=0)
loop in pthreadpool_prepare_pool deterministically terminates. It's
probably not really supported right now, but what if a helper thread
creates new jobs? This might keep the helper threads busy, and they
might re-enter idle state over and over again when prepare_pool checks
for idle threads. More parallelism is good, your patch is of course
simpler and thus preferrable, but I would feel better if we had a
"proof" that prepare_pool never runs into an endless loop. Keep in
mind that every single small race we open will eventually be hit at a
customer installation :-)

Thanks,

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de