git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 0/5] Retry if fdopen() fails due to ENOMEM

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Mar 10, 2015, 11:42 UTC
Message-ID
<54FED8C0.7060104@alum.mit.edu>
In-Reply-To
<xmqqy4nb2qwn.fsf@gitster.dls.corp.google.com>
On 03/05/2015 08:19 PM, Junio C Hamano wrote:
Show 22 quoted lines
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> One likely reason for fdopen() to fail is the lack of memory for
>> allocating a FILE structure. When that happens, try freeing some
>> memory and calling fdopen() again in the hope that it will work the
>> second time.
> 
> In codepaths where we are likely under memory pressure, the above
> might help, but I have to wonder
> 
>     (1) if update-server-info and daemon fall into that category; and
> 
>     (2) if Git continues to work under such a memory pressure to
>         cause even fdopen() to fail.
> 
> In other words, I do not see a reason not to do this change, but I
> am not sure how much it would help us in practice.
> 
> We call fopen() from a lot more places than we call fdopen().  Do we
> want to do the same, or is there a good reason why this does not
> matter to callers of fopen(), and if so why doesn't the same reason
> apply to callers of fdopen()?

To be honest, I was hoping that Jonathan would jump in and respond to this thread, as he is the one who suggested this change.

I agree that it seems rather unlikely that a call to fdopen() is the straw that breaks the camel's back. But I've never looked very closely at Git's facility for freeing up memory when an allocation fails and don't really have a mental model for how it is used.

If memory is allocated profligately under the expectation that it can be freed if necessary (i.e., if calls to try_to_free_routine() are relatively routine and tend to free a lot of memory), then it seems important that as many memory allocation paths as possible call it and retry when there is an error. This would be the garbage-collection model, in which a failure to call the garbage collector at the right time unnecessarily turns a routine event into a fatal error.

On the other hand, if calling try_to_free_routine() is an act of desperation and typically results in little or no memory being freed, then calling it in this code path is probably not very useful.

You also make a good point that if this rigmarole makes sense for fdopen(), then it probably also makes sense for fopen().

Michael
-- 
Michael Haggerty
mhagger@alum.mit.edu
Previous: Junio C HamanoNext: Torsten Bögershausen
Message 10 of 13 in “Retry if fdopen() fails due to ENOMEM”
  1. 0/5 Retry if fdopen() fails due to ENOMEMMichael Haggerty, Mar 5, 2015
  2. 1/5 xfdopen(): if first attempt fails, free memory and try againMichael Haggerty, Mar 5, 2015
  3. Stefan BellerMar 5, 2015
  4. Junio C HamanoMar 5, 2015
  5. 2/5 fdopen_lock_file(): use fdopen_with_retry()Michael Haggerty, Mar 5, 2015
  6. 3/5 copy_to_log(): use fdopen_with_retry()Michael Haggerty, Mar 5, 2015
  7. 4/5 update_info_file(): use fdopen_with_retry()Michael Haggerty, Mar 5, 2015
  8. 5/5 buffer_fdinit(): use fdopen_with_retry()Michael Haggerty, Mar 5, 2015
  9. Junio C HamanoMar 5, 2015
  10. Michael HaggertyMar 10, 2015
  11. Torsten BögershausenMar 6, 2015
  12. Michael HaggertyMar 10, 2015
  13. Junio C HamanoMar 17, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.