[PATCH] Fix a pthreadpool race

Ralph Wuerthner ralphw at de.ibm.com
Fri Dec 15 16:43:26 UTC 2017


On 15.12.2017 17:09, jim via samba-technical wrote:
> On 12/15/2017 10:57 AM, Ralph Wuerthner via samba-technical wrote:
>> On 15.12.2017 12:54, Volker Lendecke wrote:
>>> 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 :-)
>>
>> I see, this is a valid concern. Let me check if I can harden the code 
>> to guarantee that pthreadpool_prepare_pool will not end in an endless 
>> loop.
> 
> You could add a loop counter with a limit value test to generate an 
> error if exceeded.

I don't think that this is the right path. You will see this error, but 
pthreadpool_prepare_pool() is still stuck and cannot continue because 
there are idle threads sitting on pool->condvar. You could only escape 
via a process exit, but this could lead to corrupted customer files.

I already started to work on this and on Monday I should have an 
improved version available.

-- 
Regards

    Ralph Wuerthner




More information about the samba-technical mailing list