Skip to content

Commit 09275ad

Browse files
committed
Index and PackFiles do not obtain a lock anymore before reading the files in question - this won't work on read-only alternate repositories anyway, and shouldn't be necessary considering a pack is immutable
1 parent 9e6e7d7 commit 09275ad

3 files changed

Lines changed: 27 additions & 13 deletions

File tree

db/loose.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
)
2424

2525
from gitdb.util import (
26+
file_contents_ro_filepath,
2627
ENOENT,
2728
to_hex_sha,
2829
hex_to_bin,
@@ -100,12 +101,12 @@ def _map_loose_object(self, sha):
100101
:raise BadObject: if object could not be located"""
101102
db_path = self.db_path(self.object_path(to_hex_sha(sha)))
102103
try:
103-
fd = os.open(db_path, os.O_RDONLY|self._fd_open_flags)
104+
return file_contents_ro_filepath(db_path, flags=self._fd_open_flags)
104105
except OSError,e:
105106
if e.errno != ENOENT:
106107
# try again without noatime
107108
try:
108-
fd = os.open(db_path, os.O_RDONLY)
109+
return file_contents_ro_filepath(db_path)
109110
except OSError:
110111
raise BadObject(to_hex_sha(sha))
111112
# didn't work because of our flag, don't try it again

pack.py

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,9 @@
55
)
66
from util import (
77
zlib,
8-
LockedFD,
98
LazyMixin,
109
unpack_from,
11-
file_contents_ro,
10+
file_contents_ro_filepath,
1211
)
1312

1413
from fun import (
@@ -140,10 +139,10 @@ def _set_cache_(self, attr):
140139
elif attr == "_packfile_checksum":
141140
self._packfile_checksum = self._data[-20:]
142141
elif attr == "_data":
143-
lfd = LockedFD(self._indexpath)
144-
fd = lfd.open()
145-
self._data = file_contents_ro(fd)
146-
lfd.rollback()
142+
# Note: We don't lock the file when reading as we cannot be sure
143+
# that we can actually write to the location - it could be a read-only
144+
# alternate for instance
145+
self._data = file_contents_ro_filepath(self._indexpath)
147146
else:
148147
# now its time to initialize everything - if we are here, someone wants
149148
# to access the fanout table or related properties
@@ -337,10 +336,7 @@ def __init__(self, packpath):
337336

338337
def _set_cache_(self, attr):
339338
if attr == '_data':
340-
ldb = LockedFD(self._packpath)
341-
fd = ldb.open()
342-
self._data = file_contents_ro(fd)
343-
ldb.rollback()
339+
self._data = file_contents_ro_filepath(self._packpath)
344340

345341
# read the header information
346342
type_id, self._version, self._size = unpack_from(">4sLL", self._data, 0)

util.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,24 @@ def file_contents_ro(fd, stream=False, allow_mmap=True):
114114
if stream:
115115
return cStringIO.StringIO(contents)
116116
return contents
117-
117+
118+
def file_contents_ro_filepath(filepath, stream=False, allow_mmap=True, flags=0):
119+
"""Get the file contents at filepath as fast as possible
120+
:return: random access compatible memory of the given filepath
121+
:param stream: see ``file_contents_ro``
122+
:param allow_mmap: see ``file_contents_ro``
123+
:param flags: additional flags to pass to os.open
124+
:raise OSError: If the file could not be opened
125+
:note: for now we don't try to use O_NOATIME directly as the right value needs to be
126+
shared per database in fact. It only makes a real difference for loose object
127+
databases anyway, and they use it with the help of the ``flags`` parameter"""
128+
fd = os.open(filepath, os.O_RDONLY|flags)
129+
try:
130+
return file_contents_ro(fd, stream, allow_mmap)
131+
finally:
132+
close(fd)
133+
# END assure file is closed
134+
118135
def to_hex_sha(sha):
119136
""":return: hexified version of sha"""
120137
if len(sha) == 40:

0 commit comments

Comments
 (0)