From: Junio C Hamano <gitster@pobox.com>
To: Matheus Tavares <matheus.bernardino@usp.br>
Cc: git@vger.kernel.org, phil.hord@gmail.com, dstolee@microsoft.com,
jonathantanmy@google.com, stefanbeller@gmail.com
Subject: Re: [PATCH 1/2] packfile: fix race condition on unpack_entry()
Date: Mon, 28 Sep 2020 11:05:52 -0700 [thread overview]
Message-ID: <xmqqpn65n5dr.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <42a7948f94cb57ebd9c37c3850b46b1ac9813ec6.1601311803.git.matheus.bernardino@usp.br> (Matheus Tavares's message of "Mon, 28 Sep 2020 13:50:34 -0300")
Matheus Tavares <matheus.bernardino@usp.br> writes:
> To fix the race condition (and later segmentation fault), let's reorder
> the aforementioned steps so that the lock is not released between 1.
> and 3.
In other words, we hold the base in core only for ourselves without
adding it to the base cache, apply the delta to produce the result
and then place the base in the cache, and the reason why this change
fixes the breakage is because the base we have locally and not in
cache will not be seen by other people and will not be freed without
our consent? Which does make sense.
But I was confused by the explanation "lock is not released". We do
release the same lock when we call unpack_compressed_entry(), and
reaquire it before the unpack_compressed_entry() returns. What the
reordering achieves is to protect the base from getting evicted when
the unlocking and relocing happens, no?
Thanks.
next prev parent reply other threads:[~2020-09-28 18:06 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-25 2:36 RFC - concurrency causes segfault in git grep since 2.26.0 Phil Hord
2020-09-25 5:52 ` Matheus Tavares
2020-09-25 19:53 ` Phil Hord
2020-09-28 16:50 ` [PATCH 0/2] Fix race condition and memory leak in delta base cache Matheus Tavares
2020-09-28 16:50 ` [PATCH 1/2] packfile: fix race condition on unpack_entry() Matheus Tavares
2020-09-28 18:05 ` Junio C Hamano [this message]
2020-09-28 16:50 ` [PATCH 2/2] packfile: fix memory leak in add_delta_base_cache() Matheus Tavares
2020-09-28 18:22 ` Junio C Hamano
2020-09-29 0:01 ` [PATCH v2 0/2] Fix race condition and memory leak in delta base cache Matheus Tavares
2020-09-29 0:01 ` [PATCH v2 1/2] packfile: fix race condition on unpack_entry() Matheus Tavares
2020-10-02 20:06 ` Phil Hord
2020-09-29 0:01 ` [PATCH v2 2/2] packfile: fix memory leak in add_delta_base_cache() Matheus Tavares
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqpn65n5dr.fsf@gitster.c.googlers.com \
--to=gitster@pobox.com \
--cc=dstolee@microsoft.com \
--cc=git@vger.kernel.org \
--cc=jonathantanmy@google.com \
--cc=matheus.bernardino@usp.br \
--cc=phil.hord@gmail.com \
--cc=stefanbeller@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).