Web lists-archives.com

Re: [PATCH 02/11] treewide: prefer lockfiles on the stack




On 2 October 2017 at 05:37, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Martin Ågren <martin.agren@xxxxxxxxx> writes:
>
> This is not a show-stopper for this patch series, but just something
> I noticed, something that used to be unavoidable in the old world
> order that requires us to leak lockfiles, but now it becomes more
> obvious.
>
> update_index_if_able() calls write_lock_index() with the COMMIT_LOCK
> option, but it does so conditionally.  When it does not make the call,
> the lockfile is left behind to be cleaned up by the atexit() handler,
> but when it does, it is closed and released.
>
> Perhaps update_index_if_able() needs to be further cleaned up to
> always release the resource held by the lockfile structure?  Its
> caller cannot differentiate (and more importantly, its caller does
> not want to care) if the _if_able call actually has updated the
> index or not and act differently afterwards.

Ah, indeed. I should be able to do that rather easily on top. I tend to
agree that the caller shouldn't have to worry about it since the
function is just a one-off "if_able" without any return value. One
caller actually does roll back the lock, but the others don't.