Mark,

My 2 cents: Isn't it more appropriate to set FD_CLOEXEC on the fd?

        fcntl(fd, F_SETFD, FD_CLOEXEC); 

It doesn't sound like you ever want to have a cached connection be copied into 
the child. Mum and child calling daddy using the same phone line isn't going to 
make the conversation any easier for daddy.

Cheers,

Nick

On 25 Oct 2010, at 01:16, Mark Johnston wrote:

> Hello,
> 
> We've run into problems with pkg_add because of some caching behaviour
> in libfetch. Specifically, after an FTP transfer, a connection to the
> FTP server is held open by the cached_connection pointer in ftp.c. Thus,
> if one requests a file with fetchGetFTP() and later closes the
> connection with fclose(), a socket is still held open, and the
> descriptor is copied to any child processes.
> 
> What was apparently happening was that we were using pkg_add to install
> a package whose install script started a daemon, which consequently kept
> open a connection to our FTP server. This is "fixed" in our tree with a
> closefrom(2) in pkg_install at an appropriate point, but I thought that
> libfetch should provide some way of forcing a close on the cached
> connection so that the above hack isn't necessary.
> 
> My solution is provided in a patch below. It's not particularly elegant,
> but I can't see a better way to go about it. I was hoping to get some
> feedback and to see if anyone can come up with a better approach. I'll
> also update the libfetch man page if the patch below is acceptable.
> 
> Thanks,
> -Mark
> 
> diff --git a/lib/libfetch/fetch.h b/lib/libfetch/fetch.h
> index 11a3f77..d379e63 100644
> --- a/lib/libfetch/fetch.h
> +++ b/lib/libfetch/fetch.h
> @@ -109,6 +109,7 @@ FILE              *fetchGetFTP(struct url *, const char 
> *);
> FILE          *fetchPutFTP(struct url *, const char *);
> int            fetchStatFTP(struct url *, struct url_stat *, const char *);
> struct url_ent        *fetchListFTP(struct url *, const char *);
> +void          fetchCloseCachedFTP();
> 
> /* Generic functions */
> FILE          *fetchXGetURL(const char *, struct url_stat *, const char *);
> diff --git a/lib/libfetch/ftp.c b/lib/libfetch/ftp.c
> index 0983a76..746ad54 100644
> --- a/lib/libfetch/ftp.c
> +++ b/lib/libfetch/ftp.c
> @@ -1204,3 +1204,12 @@ fetchListFTP(struct url *url __unused, const char 
> *flags __unused)
>       warnx("fetchListFTP(): not implemented");
>       return (NULL);
> }
> +
> +/*
> + * Force close the cached connection.
> + */
> +void
> +fetchCloseCachedFTP()
> +{
> +     ftp_disconnect(cached_connection);
> +}
> diff --git a/usr.sbin/pkg_install/lib/url.c b/usr.sbin/pkg_install/lib/url.c
> index 914ce39..68f31bb 100644
> --- a/usr.sbin/pkg_install/lib/url.c
> +++ b/usr.sbin/pkg_install/lib/url.c
> @@ -163,5 +163,6 @@ fileGetURL(const char *base, const char *spec, int 
> keep_package)
>       printf("tar command returns %d status\n", WEXITSTATUS(pstat));
>     if (rp && (isatty(0) || Verbose))
>       printf(" Done.\n");
> +    fetchCloseCachedFTP();
>     return rp;
> }
> _______________________________________________
> freebsd-current@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Reply via email to