Michael Haggerty [off-list ref] 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()?
Michael Haggerty (5):
xfdopen(): if first attempt fails, free memory and try again
fdopen_lock_file(): use fdopen_with_retry()
copy_to_log(): use fdopen_with_retry()
update_info_file(): use fdopen_with_retry()
buffer_fdinit(): use fdopen_with_retry()
daemon.c | 4 ++--
git-compat-util.h | 11 +++++++++++
lockfile.c | 2 +-
server-info.c | 2 +-
vcs-svn/line_buffer.c | 2 +-
wrapper.c | 28 +++++++++++++++++++++++++---
6 files changed, 41 insertions(+), 8 deletions(-)
On 03/05/2015 08:19 PM, Junio C Hamano wrote:
Michael Haggerty [off-list ref] writes:
quoted
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