Re: [PATCH 1/5] xfdopen(): if first attempt fails, free memory and try again
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 5, 2015, 19:06 UTC
- Message-ID
- <xmqq4mpz463f.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1425571669-22800-2-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> One likely reason for the failure of fdopen() is a lack of free > memory.
Interesting.
Did you find this by code inspection?
Or did you actually hit this issue in real life, and applying this patch helped? The latter would indicate that this failure is rather common with your workload, and that Git can continue working even when the process is so memory starved to cause fdopen() to fail.
> Also expose a new function, fdopen_with_retry(), which retries on > ENOMEM but doesn't die() on errors. In a moment this function will be > used elsewhere.
Hmm, OK, I guess these three lines answers my question---asking that question at this point in the series is moot ;-)
The code looks good.
Show 74 quoted lines
> Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> git-compat-util.h | 11 +++++++++++
> wrapper.c | 28 +++++++++++++++++++++++++---
> 2 files changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 3455c5e..a5652a7 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -672,7 +672,18 @@ extern ssize_t xread(int fd, void *buf, size_t len);
> extern ssize_t xwrite(int fd, const void *buf, size_t len);
> extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
> extern int xdup(int fd);
> +
> +/*
> + * Like fdopen(), but if the first attempt fails with ENOMEM, try to
> + * free up some memory and try again.
> + */
> +extern FILE *fdopen_with_retry(int fd, const char *mode);
> +
> +/*
> + * Like fdopen_with_retry(), but die on errors.
> + */
> extern FILE *xfdopen(int fd, const char *mode);
> +
> extern int xmkstemp(char *template);
> extern int xmkstemp_mode(char *template, int mode);
> extern int odb_mkstemp(char *template, size_t limit, const char *pattern);
> diff --git a/wrapper.c b/wrapper.c
> index d5a6cef..b60cc03 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -311,14 +311,36 @@ int xdup(int fd)
> return ret;
> }
>
> -FILE *xfdopen(int fd, const char *mode)
> +FILE *fdopen_with_retry(int fd, const char *mode)
> {
> FILE *stream = fdopen(fd, mode);
> - if (stream == NULL)
> - die_errno("Out of memory? fdopen failed");
> +
> + if (!stream && errno == ENOMEM) {
> + /*
> + * Try to free up some memory, then try again. We
> + * would prefer to use sizeof(FILE) here, but that is
> + * not guaranteed to be defined (e.g., FILE might be
> + * an incomplete type).
> + */
> + try_to_free_routine(1000);
> + stream = fdopen(fd, mode);
> + }
> +
> return stream;
> }
>
> +FILE *xfdopen(int fd, const char *mode)
> +{
> + FILE *stream = fdopen_with_retry(fd, mode);
> +
> + if (stream)
> + return stream;
> + else if (errno == ENOMEM)
> + die_errno("Out of memory? fdopen failed");
> + else
> + die_errno("fdopen failed");
> +}
> +
> int xmkstemp(char *template)
> {
> int fd;