John Dalbec wrote:
> Nico Erfurth wrote:
>
>> John Dalbec wrote:
>>
>>
>>>> IIRC exim calls the tidyup-function of all lookuptypes before
>>>> spawning a
>>>> new process, so this shouldn't happen as long as ldap_tidyup is
>>>> implemented correctly.
>>>
>>>
>>> I don't think that's happening in rda.c:
>>
>>
>>
>> Hmm, right.
>>
>>
>>> /* Ensure that SIGCHLD is set to SIG_DFL before forking, so that the
>>> child
>>> process can be waited for. We sometimes get here with it set otherwise.
>>> Save
>>> the old state for resetting on the wait. */
>>
>>
>>
>> search_tidyup(); <<<< Add this
>>
>>
>>> oldsignal = signal(SIGCHLD, SIG_DFL);
>>> if ((pid = fork()) == 0)
>>
>>
>>
>>
>>> I see nothing about tidying up in this code. What should I add?
>>
>>
>>
>> The search_tidyup should free all lookup resources the current process
>> uses. The problem is, that the main-process will also delete the whole
>> lookup cache, that could result in a bad performance.
>>
>> Nico
>>
>
> How about this? It hides the existing LDAP connections from the child
> process immediately after forking, then tidies any LDAP connections the
> child may have created before the child ends. The existing LDAP
> connections are then still available to the parent so it shouldn't
> impact performance as much as a full search_tidyup. Actually the while
> loop in eldap_tidy_no_unbind could be reduced to ldap_connections =
> NULL; but this way it's easier to see the connection to eldap_tidy.
> John
I ran grep -i {SSL,TLS} * in src/lookups and LDAP seems to be the only lookup
that's SSL-enabled, so tidying the other lookup types would seem to be
unnecessary here.
Actually any connection that gets created in the child process probably should
be tidied when that process exits because the parent process won't know about
it. In order to do this, the PID of the process that created the connection
needs to be stored in the cache tables.
John
>
> --- src/lookups/ldap.c.orig Tue Jul 19 10:28:59 2005
> +++ src/lookups/ldap.c Tue Jul 19 10:29:06 2005
> @@ -1222,6 +1222,18 @@
> }
> }
>
> +void
> +eldap_tidy_no_unbind(void)
> +{
> +LDAP_CONNECTION *lcp = NULL;
> +eldap_dn = NULL;
> +
> +while ((lcp = ldap_connections) != NULL)
> + {
> + ldap_connections = lcp->next;
> + }
> +}
> +
>
>
> /*************************************************
> --- src/lookups/ldap.h.orig Tue Jul 19 10:30:29 2005
> +++ src/lookups/ldap.h Tue Jul 19 10:30:34 2005
> @@ -19,6 +19,7 @@
> extern int eldapm_find(void *, uschar *, uschar *, int, uschar **,
> uschar **, BOOL *);
> extern void eldap_tidy(void);
> +extern void eldap_tidy_no_unbind(void);
> extern uschar *eldap_quote(uschar *, uschar *);
>
> /* End of lookups/ldap.h */
> --- src/rda.c.orig Tue Jul 19 10:23:19 2005
> +++ src/rda.c Tue Jul 19 10:38:17 2005
> @@ -614,6 +614,7 @@
> {
> header_line *waslast = header_last; /* Save last header */
>
> + (void)eldap_tidy_no_unbind();
> fd = pfd[pipe_write];
> (void)close(pfd[pipe_read]);
> exim_setugid(ugid->uid, ugid->gid, FALSE, rname);
> @@ -743,6 +744,7 @@
> /* OK, this process is now done. Must use _exit() and not exit() !! */
>
> (void)close(fd);
> + (void)eldap_tidy();
> _exit(0);
> }
>
>
>
