[PATCH] clean the event context after fork in fork_domain_child()

Jeremy Allison jra at samba.org
Tue Jan 6 04:28:27 GMT 2009


On Tue, Jan 06, 2009 at 12:50:12PM +0800, boyang wrote:

> 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 know it causes a memory leak, but it can't lead to
double free. Remember, Metze's patch removes the
event pointers from the linked list, but doesn't
free them. So even if some codepath has a saved
pointer and calls TALLOC_FREE() on it there won't
be a double free, the saved pointer hasn't been
freed. That's why it causes a memory leak.

Doing it this way is safer than trying to free
the event pointers inside the event code, that
way can lead to crashes if the saved pointer
is referenced.

When I've added the code to convert the
reinit_after_fork() calls to the individual
winbindd_reinit_after_fork() etc., this
won't be a problem as part of the 
winbindd_reinit_after_fork() call will be
code to free all pending events and null out
their saved pointers, so the event_context_reinit()
should be a no-op (as all the events on the
list will already have been freed).

> 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.

Please send asap so I can evaluate it as I
back-port Metze's code to 3.3-test.

Thanks,

Jeremy.


More information about the samba-technical mailing list