← Back to team overview

dulwich-users team mailing list archive

Re: [PATCH] Race condition in DiskObjectStore.add_object

 

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!

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.

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.


Attachment: signature.asc
Description: OpenPGP digital signature


Follow ups

References