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
