[bitbake-devel] [PATCH] fetch2/gitsm: fix config file concurrent update race

[Ming Liu]

Hi, Richard:

Involve in Mark.

Please correct me if I am wrong, but according to the code, seems these
upper level locks could not protect this race condition, because it's
inside the GitSM unpack function, the call chain is:

Inside GitSM:
```
def unpack
  -> for module in submodules:
       -> unpack_submodules
            -> runfetchcmd
                 -> bb.process.run
                      -> git config submodule
```

since 'git config submodule' run concurrently and try to modify a same git
config file, hence the race might happen, but it's hard to be reproduced
since 'git config submodule' finishes quite quickly.

//Ming Liu

Richard Purdie <richard.purdie at linuxfoundation.org> 於 2019年1月25日 週五
下午1:24寫道：

> On Fri, 2019-01-25 at 12:35 +0100, Ming Liu wrote:
> > Hi, Stefan:
> >
> > Could you help confirm this?
> >
> > Hi, Richard:
> >
> > May I know how has the issue been fixed?
> >
> > I updated the tip to:
> > ```
> > commit 8c8ecec2a722bc2885e2648d41ac8df07bdf660d
> >     gitsmy.py: Fix unpack of submodules of submodules
> >
> >     If the submodule is in a subdirectory, it needs to have that
> > structure
> >     preserved.  This means the unpack path needs to be in the
> > 'dirname' of the
> >     final path -- since the unpack directory name is specified in the
> > URI.
> >
> >     Additional specific test cases were added to ensure this is
> > working properly
> >     based on two recent error reports.
> >
> >     Signed-off-by: Mark Hatle <mark.hatle at windriver.com>
> >     Signed-off-by: Richard Purdie <richard.purdie at linuxfoundation.org
> > >
> > ```
> > on top of that commit, I could see there are "git config submodule"
> > being called concurrently in different processes, which does have a
> > race condition, and will lead to the error observed by Stefan,
> > although it's not easy to be reproduced.
>
> There are locks at a higher level in the fetchers, in
> lib/bb/fetch2/__init__.py the download() method has:
>
> if ud.lockfile:
>     lf = bb.utils.lockfile(ud.lockfile)
>
> and the unpack() method has similar locks.
>
> Mark and I believe there were some code paths where the locks were
> being bypassed in the gitsm fetcher but Mark reworked the code so that
> we were both happy that there should always be a lock held.
>
> We need to fix any races properly ensuring they're covered by the main
> fetcher locks, adding new ones would only fix a symptom of the real
> problem.
>
> Cheers,
>
> Richard
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openembedded.org/pipermail/bitbake-devel/attachments/20190125/6900b0c9/attachment.html>