Re: [PATCH 1/7] refs/reftable: reload correct stack when creating reflog iter

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1725 bytes --]

On Mon, Mar 11, 2024 at 11:34:23AM -0700, Josh Steadmon wrote:
> On 2024.03.05 13:10, Patrick Steinhardt wrote:
> > When creating a new reflog iterator, we first have to reload the stack
> > that the iterator is being created. This is done so that any concurrent
> > writes to the stack are reflected. But `reflog_iterator_for_stack()`
> > always reloads the main stack, which is wrong.
> > 
> > Fix this and reload the correct stack.
> > 
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  refs/reftable-backend.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> > index 249a618b5a..f04be942ac 100644
> > --- a/refs/reftable-backend.c
> > +++ b/refs/reftable-backend.c
> > @@ -1682,7 +1682,7 @@ static struct reftable_reflog_iterator *reflog_iterator_for_stack(struct reftabl
> >  	if (ret)
> >  		goto done;
> >  
> > -	ret = reftable_stack_reload(refs->main_stack);
> > +	ret = reftable_stack_reload(stack);
> >  	if (ret < 0)
> >  		goto done;
> >  
> > -- 
> > 2.44.0
> > 
> 
> Is it possible to write a test to demonstrate the bug that was fixed
> here, or is it too much of a race condition to reliably trigger?

I wouldn't really know how to test for this in a way that is even
somewhat reliably, unfortunately. You have to have at least two
concurrent commands, one reading and one writing, where the first
command loads the worktree stack and then tries to create an iter as
above.

Now if this was part of the reftable library we could do it rather
easily via t0032, which contains unit tests for the reftable library.
But in the reftable backend it's much harder.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]