← Back to team overview

dulwich-users team mailing list archive

Re: [PATCH] Race condition in DiskObjectStore.add_object

 

On Thu, Mar 25, 2010 at 10:36, Bruce Duncan <Bruce.Duncan@xxxxxxxx> wrote:

> Hi David,
>
> > Thanks for the patch. It may be worth just grepping for os.mkdir to see
> > if there are other potential race conditions that match this pattern and
> > maybe fix them all in one sweep.
>
> I had a quick look for other race conditions. The only other place where
> os.mkdir appears is in the Repo.init and init_bare methods. I'm not sure
> what should be the defined behaviour if two threads try to init a repo
> at the same place!
>

Yeah, I don't think that's worth worrying about. cgit probably has some
defined behavior for that because they're really careful about all sorts of
race conditions, but it's certainly beyond the scope of your previous change
:)


> I looked for os.path.exists and os.path.isdir, too. I think there is a
> potential race condition in Repo.open_index, but I can't see what uses it.
>
> We have hit another race condition where
> self.repo.refs['refs/heads/master'] ends up being "", but that's
> probably in my code.
>

Let me know if you need some help debugging this. I recently rewrote much of
the DiskRefsContainer code to avoid race conditions, so it's possible this
is my fault.


> Bruce
>
> > Dave
> >
> > On Tue, Mar 23, 2010 at 04:33, Bruce Duncan <Bruce.Duncan@xxxxxxxx
> > <mailto:Bruce.Duncan@xxxxxxxx>> wrote:
> >
> >     Hi,
> >
> >     We are using dulwich in a project. It is good!
> >
> >     Our daemon is threaded and we have managed to hit a race condition
> while
> >     creating objects. I believe the attached patch fixes this. Please can
> >     you apply it?
> >
> >     Bruce
> >
> >     In threaded programs, more than one thread can try to add the same
> >     object at
> >     the same time and this can cause os.mkdir to fail.
> >     ---
> >      dulwich/object_store.py |    5 ++++-
> >      1 files changed, 4 insertions(+), 1 deletions(-)
> >
> >     diff --git a/dulwich/object_store.py b/dulwich/object_store.py
> >     index ca26348..46832f5 100644
> >     --- a/dulwich/object_store.py
> >     +++ b/dulwich/object_store.py
> >     @@ -470,8 +470,11 @@ class DiskObjectStore(PackBasedObjectStore):
> >             :param obj: Object to add
> >             """
> >             dir = os.path.join(self.path, obj.id <http://obj.id>[:2])
> >     -        if not os.path.isdir(dir):
> >     +        try:
> >                 os.mkdir(dir)
> >     +        except OSError, e:
> >     +            if e.errno != errno.EEXIST:
> >     +                raise
> >             path = os.path.join(dir, obj.id <http://obj.id>[2:])
> >             if os.path.exists(path):
> >                 return # Already there, no need to write again
> >     --
> >     1.6.3.3
> >
> >     --
> >     CLX Compute Cluster Developer (http://clx.see.ed.ac.uk/)
> >     AL116, Alrick Building, King's Buildings. 0131 6505637
> >     School of Engineering, University of Edinburgh
> >
> >     The University of Edinburgh is a charitable body, registered in
> >     Scotland, with registration number SC005336.
> >
> >
> >     _______________________________________________
> >     Mailing list: https://launchpad.net/~dulwich-users
> >     <https://launchpad.net/%7Edulwich-users>
> >     Post to     : dulwich-users@xxxxxxxxxxxxxxxxxxx
> >     <mailto:dulwich-users@xxxxxxxxxxxxxxxxxxx>
> >     Unsubscribe : https://launchpad.net/~dulwich-users
> >     <https://launchpad.net/%7Edulwich-users>
> >     More help   : https://help.launchpad.net/ListHelp
> >
> >
>
> --
> CLX Compute Cluster Developer (http://clx.see.ed.ac.uk/)
> AL116, Alrick Building, King's Buildings. 0131 6505637
> School of Engineering, University of Edinburgh
>
> The University of Edinburgh is a charitable body, registered in
> Scotland, with registration number SC005336.
>
>
>

References