[PATCH] clean the event context after fork
boyang at suse.de
Tue Jan 6 04:50:12 GMT 2009
Jeremy Allison wrote:
> On Mon, Jan 05, 2009 at 02:46:07PM +0100, Stefan (metze) Metzmacher wrote:
>> boyang schrieb:
>>> Stefan (metze) Metzmacher wrote:
>>>> Hi BoYang,
>>>>> The point here is that there maybe outstanding requests in parent,
>>>>> and async_request_timeout is set, which cause this child try to kill
>>>>> some other child, probably be primary domain's child. Look, After
>>>>> winbindd started, child for primary domain is forked, then other domains
>>>>> such as BUILTIN. If there are outstanding requests sent to child for
>>>>> primary domain, async_request_timeout handler is set, which is
>>>>> inheritted in child for domain BUILTIN. Later, time expired and child
>>>>> for BUILTIN sends SIGTERM to child for primary domain.
>>>>> Patches are in the attachment, Thanks!
>>>> I think the real solution is to create a new event context in the child
>>>> process (and free the parents context). In the end we should do that
>>>> also within smbd...
>>> Hi, Metze:
>>> I've finished the patch to clean event context after fork. Please
>>> review it. In event_context_reinit(ev), I freed all timed_event and
>>> fd_event to save some memory. Initial test and 'make test' looks fine.
>> I think freeing the events is dangerous, as it leaves broken pointers
> Yes, I'm planning to add code to go through and clean up
> any events left registered after each fork, and change
> reinit_after_fork() to smbd_reinit_after_fork(), nmbd_reinit_after_fork()
> and winbindd_reinit_after_fork() to make sure each cleans
> up correctly.
> I agree with your assessment that leaving the broken pointers
> around is dangerous.
This can lead to double free. That is why I set
domain->check_online_event to NULL(entry->event have to be set too).
Since we didn't free it now, we can not barely set check_online_event to
NULL, which causes memory leak. We must free it after
reinit_after_fork() is called. I'll post the patch later.
>> I integrated it without freeing the events and your krb5 ticket patch
>> in this branch:
>> It would be nice if you could test if that branch fixes still fixes all
>> the problems you wanted to resolve.
Looks like ccache_regain_all_now relies on entry->refresh_time to judge
whether the handler is krb5_ticket_refresh_handler or
krb5_ticket_gain_handler. Therefore, we must set entry->refresh_time
before event_add_timed to make ccache_regain_all_now work correctly.
I'll post a separate patch later.
> These fixes are important enough to delay 3.3.0 for, IMHO.
> Thanks Metze and Boyang, really nice work here !
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 187 bytes
Desc: not available
Url : http://lists.samba.org/archive/samba-technical/attachments/20090106/9d4e306e/boyang.vcf
More information about the samba-technical