On 12/19/2016 02:04 PM, thierry bordaz wrote:
> Hi Tomas,
>
> The patch looks good to me.
> Just a minor remark.
> ldap_inst->exiting=TRUE and signaling the watcher thread is the same
> action. Ideally the signal handler would set 'existing=TRUE', but
> there is no nice way for the signal handler to retrieve/set the
> 'existing' flag. Do you think we could move 'ldap_inst->exiting=TRUE'
> and pthread_kill in a same wrapper function (for example
> watcher_shutdown).
>
> thanks
> thierry
>
> On 12/19/2016 01:04 PM, Tomas Krizek wrote:
>> Hi Thierry,
>>
>> could you please take a look at this bind-dyndb-ldap patch? I was trying
>> to fix https://fedorahosted.org/bind-dyndb-ldap/ticket/149
>>
>> I wasn't able to reproduce the issue, but I think the problem is fixed
>> now. Petr Spacek was helping me with this, but I think it would be good
>> if you could take a look as well.
>>
>> We were able to identify two causes: a) isc_thread_create fails and
>> ldap_inst->watcher is undefined (fixed by setting it to 0) and b) the
>> thread terminates for some reason (i.e. some checks fail) and then a
>> signal can't be sent to it. This was addressed by removing the REQUIRE
>> and logging an error instead.
>>
>> Thanks.
>>
>
Moved the ldap_inst-> exiting and pthread_kill to a wrapper function.

Please continue the review at
https://github.com/freeipa/bind-dyndb-ldap/pull/6

-- 
Tomas Krizek

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to