Date: Fri, 5 Nov 2010 09:38:24 +0100 From: Nick Hibma <nick@van-laarhoven.org> To: Mark Johnston <markjdb@gmail.com> Cc: freebsd-current@freebsd.org Subject: Re: [Patch] libfetch - closing the cached FTP connection Message-ID: <A72115A0-00E4-4FDB-974C-E06D31116794@van-laarhoven.org> In-Reply-To: <20101024231642.GB2123@mark-laptop-bsd.mark-home> References: <20101024231119.GA2123@mark-laptop-bsd.mark-home> <20101024231642.GB2123@mark-laptop-bsd.mark-home>
next in thread | previous in thread | raw e-mail | index | archive | help
Mark,
My 2 cents: Isn't it more appropriate to set FD_CLOEXEC on the fd?
fcntl(fd, F_SETFD, FD_CLOEXEC);=20
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,
>=20
> 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.
>=20
> 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.
>=20
> 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.
>=20
> Thanks,
> -Mark
>=20
> 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();
>=20
> /* 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-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?A72115A0-00E4-4FDB-974C-E06D31116794>