[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