Skip to content

Commit 8fdefdd

Browse files
alexvy86isomorphic-git-bot
alexvy86
authored and
isomorphic-git-bot
committed
Fix race condition when reading/writing refs (#1882)
* docs: add @alexvy86 as a contributor * Add locking to fix race condition reading/writing refs * Make prettier happy * Encapsulate locking logic * Use withLock helper * Indicate private functions * Remove obsolete variable declaration * Await all uses of RefLock.withLock * Unit test * Unit test * Use async-lock package * Fix tree-shaking
1 parent ba54ace commit 8fdefdd

File tree

6 files changed

+89
-38
lines changed

6 files changed

+89
-38
lines changed

‎js/isomorphic-git/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,7 @@ Thanks goes to these wonderful people ([emoji key](https://github.com/kentcdodds
368368
<td align="center"><a href="https://api.github.com/users/hisco"><img src="https://avatars.githubusercontent.com/u/39222286?v=4?s=60" width="60px;" alt=""/><br /><sub><b>Eyal Hisco</b></sub></a><br /><a href="https://github.com/isomorphic-git/isomorphic-git/issues?q=author%3Ahisco" title="Bug reports">🐛</a></td>
369369
<td align="center"><a href="https://github.com/scolladon"><img src="https://avatars.githubusercontent.com/u/522422?v=4?s=60" width="60px;" alt=""/><br /><sub><b>Sebastien</b></sub></a><br /><a href="https://github.com/isomorphic-git/isomorphic-git/commits?author=scolladon" title="Code">💻</a></td>
370370
<td align="center"><a href="https://github.com/yarikoptic"><img src="https://avatars.githubusercontent.com/u/39889?v=4?s=60" width="60px;" alt=""/><br /><sub><b>Yaroslav Halchenko</b></sub></a><br /><a href="https://github.com/isomorphic-git/isomorphic-git/commits?author=yarikoptic" title="Documentation">📖</a></td>
371+
<td align="center"><a href="https://alex-v.blog/"><img src="https://avatars.githubusercontent.com/u/716334?v=4?s=60" width="60px;" alt=""/><br /><sub><b>Alex Villarreal</b></sub></a><br /><a href="https://github.com/isomorphic-git/isomorphic-git/commits?author=alexvy86" title="Code">💻</a></td>
371372
</tr>
372373
</table>
373374

‎js/isomorphic-git/index.cjs‎

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1796,6 +1796,13 @@ const refpaths = ref => [
17961796
// @see https://git-scm.com/docs/gitrepository-layout
17971797
const GIT_FILES = ['config', 'description', 'index', 'shallow', 'commondir'];
17981798

1799+
let lock$1;
1800+
1801+
async function acquireLock(ref, callback) {
1802+
if (lock$1 === undefined) lock$1 = new AsyncLock();
1803+
return lock$1.acquire(ref, callback)
1804+
}
1805+
17991806
class GitRefManager {
18001807
static async updateRemoteRefs({
18011808
fs,
@@ -1902,7 +1909,9 @@ class GitRefManager {
19021909
// are .git/refs/remotes/origin/refs/remotes/remote_mirror_3059
19031910
// and .git/refs/remotes/origin/refs/merge-requests
19041911
for (const [key, value] of actualRefsToWrite) {
1905-
await fs.write(join(gitdir, key), `${value.trim()}\n`, 'utf8');
1912+
await acquireLock(key, async () =>
1913+
fs.write(join(gitdir, key), `${value.trim()}\n`, 'utf8')
1914+
);
19061915
}
19071916
return { pruned }
19081917
}
@@ -1913,11 +1922,15 @@ class GitRefManager {
19131922
if (!value.match(/[0-9a-f]{40}/)) {
19141923
throw new InvalidOidError(value)
19151924
}
1916-
await fs.write(join(gitdir, ref), `${value.trim()}\n`, 'utf8');
1925+
await acquireLock(ref, async () =>
1926+
fs.write(join(gitdir, ref), `${value.trim()}\n`, 'utf8')
1927+
);
19171928
}
19181929

19191930
static async writeSymbolicRef({ fs, gitdir, ref, value }) {
1920-
await fs.write(join(gitdir, ref), 'ref: ' + `${value.trim()}\n`, 'utf8');
1931+
await acquireLock(ref, async () =>
1932+
fs.write(join(gitdir, ref), 'ref: ' + `${value.trim()}\n`, 'utf8')
1933+
);
19211934
}
19221935

19231936
static async deleteRef({ fs, gitdir, ref }) {
@@ -1928,7 +1941,9 @@ class GitRefManager {
19281941
// Delete regular ref
19291942
await Promise.all(refs.map(ref => fs.rm(join(gitdir, ref))));
19301943
// Delete any packed ref
1931-
let text = await fs.read(`${gitdir}/packed-refs`, { encoding: 'utf8' });
1944+
let text = await acquireLock('packed-refs', async () =>
1945+
fs.read(`${gitdir}/packed-refs`, { encoding: 'utf8' })
1946+
);
19321947
const packed = GitPackedRefs.from(text);
19331948
const beforeSize = packed.refs.size;
19341949
for (const ref of refs) {
@@ -1938,7 +1953,9 @@ class GitRefManager {
19381953
}
19391954
if (packed.refs.size < beforeSize) {
19401955
text = packed.toString();
1941-
await fs.write(`${gitdir}/packed-refs`, text, { encoding: 'utf8' });
1956+
await acquireLock('packed-refs', async () =>
1957+
fs.write(`${gitdir}/packed-refs`, text, { encoding: 'utf8' })
1958+
);
19421959
}
19431960
}
19441961

@@ -1957,7 +1974,7 @@ class GitRefManager {
19571974
return ref
19581975
}
19591976
}
1960-
let sha;
1977+
19611978
// Is it a ref pointer?
19621979
if (ref.startsWith('ref: ')) {
19631980
ref = ref.slice('ref: '.length);
@@ -1973,9 +1990,12 @@ class GitRefManager {
19731990
const allpaths = refpaths(ref).filter(p => !GIT_FILES.includes(p)); // exclude git system files (#709)
19741991

19751992
for (const ref of allpaths) {
1976-
sha =
1977-
(await fs.read(`${gitdir}/${ref}`, { encoding: 'utf8' })) ||
1978-
packedMap.get(ref);
1993+
const sha = await acquireLock(
1994+
ref,
1995+
async () =>
1996+
(await fs.read(`${gitdir}/${ref}`, { encoding: 'utf8' })) ||
1997+
packedMap.get(ref)
1998+
);
19791999
if (sha) {
19802000
return GitRefManager.resolve({ fs, gitdir, ref: sha.trim(), depth })
19812001
}
@@ -2003,7 +2023,10 @@ class GitRefManager {
20032023
// Look in all the proper paths, in this order
20042024
const allpaths = refpaths(ref);
20052025
for (const ref of allpaths) {
2006-
if (await fs.exists(`${gitdir}/${ref}`)) return ref
2026+
const refExists = await acquireLock(ref, async () =>
2027+
fs.exists(`${gitdir}/${ref}`)
2028+
);
2029+
if (refExists) return ref
20072030
if (packedMap.has(ref)) return ref
20082031
}
20092032
// Do we give up?
@@ -2054,7 +2077,9 @@ class GitRefManager {
20542077
}
20552078

20562079
static async packedRefs({ fs, gitdir }) {
2057-
const text = await fs.read(`${gitdir}/packed-refs`, { encoding: 'utf8' });
2080+
const text = await acquireLock('packed-refs', async () =>
2081+
fs.read(`${gitdir}/packed-refs`, { encoding: 'utf8' })
2082+
);
20582083
const packed = GitPackedRefs.from(text);
20592084
return packed.refs
20602085
}
@@ -7216,14 +7241,14 @@ class GitRemoteManager {
72167241
}
72177242
}
72187243

7219-
let lock$1 = null;
7244+
let lock$2 = null;
72207245

72217246
class GitShallowManager {
72227247
static async read({ fs, gitdir }) {
7223-
if (lock$1 === null) lock$1 = new AsyncLock();
7248+
if (lock$2 === null) lock$2 = new AsyncLock();
72247249
const filepath = join(gitdir, 'shallow');
72257250
const oids = new Set();
7226-
await lock$1.acquire(filepath, async function() {
7251+
await lock$2.acquire(filepath, async function() {
72277252
const text = await fs.read(filepath, { encoding: 'utf8' });
72287253
if (text === null) return oids // no file
72297254
if (text.trim() === '') return oids // empty file
@@ -7236,18 +7261,18 @@ class GitShallowManager {
72367261
}
72377262

72387263
static async write({ fs, gitdir, oids }) {
7239-
if (lock$1 === null) lock$1 = new AsyncLock();
7264+
if (lock$2 === null) lock$2 = new AsyncLock();
72407265
const filepath = join(gitdir, 'shallow');
72417266
if (oids.size > 0) {
72427267
const text = [...oids].join('\n') + '\n';
7243-
await lock$1.acquire(filepath, async function() {
7268+
await lock$2.acquire(filepath, async function() {
72447269
await fs.write(filepath, text, {
72457270
encoding: 'utf8',
72467271
});
72477272
});
72487273
} else {
72497274
// No shallows
7250-
await lock$1.acquire(filepath, async function() {
7275+
await lock$2.acquire(filepath, async function() {
72517276
await fs.rm(filepath);
72527277
});
72537278
}

‎js/isomorphic-git/index.js‎

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1790,6 +1790,13 @@ const refpaths = ref => [
17901790
// @see https://git-scm.com/docs/gitrepository-layout
17911791
const GIT_FILES = ['config', 'description', 'index', 'shallow', 'commondir'];
17921792

1793+
let lock$1;
1794+
1795+
async function acquireLock(ref, callback) {
1796+
if (lock$1 === undefined) lock$1 = new AsyncLock();
1797+
return lock$1.acquire(ref, callback)
1798+
}
1799+
17931800
class GitRefManager {
17941801
static async updateRemoteRefs({
17951802
fs,
@@ -1896,7 +1903,9 @@ class GitRefManager {
18961903
// are .git/refs/remotes/origin/refs/remotes/remote_mirror_3059
18971904
// and .git/refs/remotes/origin/refs/merge-requests
18981905
for (const [key, value] of actualRefsToWrite) {
1899-
await fs.write(join(gitdir, key), `${value.trim()}\n`, 'utf8');
1906+
await acquireLock(key, async () =>
1907+
fs.write(join(gitdir, key), `${value.trim()}\n`, 'utf8')
1908+
);
19001909
}
19011910
return { pruned }
19021911
}
@@ -1907,11 +1916,15 @@ class GitRefManager {
19071916
if (!value.match(/[0-9a-f]{40}/)) {
19081917
throw new InvalidOidError(value)
19091918
}
1910-
await fs.write(join(gitdir, ref), `${value.trim()}\n`, 'utf8');
1919+
await acquireLock(ref, async () =>
1920+
fs.write(join(gitdir, ref), `${value.trim()}\n`, 'utf8')
1921+
);
19111922
}
19121923

19131924
static async writeSymbolicRef({ fs, gitdir, ref, value }) {
1914-
await fs.write(join(gitdir, ref), 'ref: ' + `${value.trim()}\n`, 'utf8');
1925+
await acquireLock(ref, async () =>
1926+
fs.write(join(gitdir, ref), 'ref: ' + `${value.trim()}\n`, 'utf8')
1927+
);
19151928
}
19161929

19171930
static async deleteRef({ fs, gitdir, ref }) {
@@ -1922,7 +1935,9 @@ class GitRefManager {
19221935
// Delete regular ref
19231936
await Promise.all(refs.map(ref => fs.rm(join(gitdir, ref))));
19241937
// Delete any packed ref
1925-
let text = await fs.read(`${gitdir}/packed-refs`, { encoding: 'utf8' });
1938+
let text = await acquireLock('packed-refs', async () =>
1939+
fs.read(`${gitdir}/packed-refs`, { encoding: 'utf8' })
1940+
);
19261941
const packed = GitPackedRefs.from(text);
19271942
const beforeSize = packed.refs.size;
19281943
for (const ref of refs) {
@@ -1932,7 +1947,9 @@ class GitRefManager {
19321947
}
19331948
if (packed.refs.size < beforeSize) {
19341949
text = packed.toString();
1935-
await fs.write(`${gitdir}/packed-refs`, text, { encoding: 'utf8' });
1950+
await acquireLock('packed-refs', async () =>
1951+
fs.write(`${gitdir}/packed-refs`, text, { encoding: 'utf8' })
1952+
);
19361953
}
19371954
}
19381955

@@ -1951,7 +1968,7 @@ class GitRefManager {
19511968
return ref
19521969
}
19531970
}
1954-
let sha;
1971+
19551972
// Is it a ref pointer?
19561973
if (ref.startsWith('ref: ')) {
19571974
ref = ref.slice('ref: '.length);
@@ -1967,9 +1984,12 @@ class GitRefManager {
19671984
const allpaths = refpaths(ref).filter(p => !GIT_FILES.includes(p)); // exclude git system files (#709)
19681985

19691986
for (const ref of allpaths) {
1970-
sha =
1971-
(await fs.read(`${gitdir}/${ref}`, { encoding: 'utf8' })) ||
1972-
packedMap.get(ref);
1987+
const sha = await acquireLock(
1988+
ref,
1989+
async () =>
1990+
(await fs.read(`${gitdir}/${ref}`, { encoding: 'utf8' })) ||
1991+
packedMap.get(ref)
1992+
);
19731993
if (sha) {
19741994
return GitRefManager.resolve({ fs, gitdir, ref: sha.trim(), depth })
19751995
}
@@ -1997,7 +2017,10 @@ class GitRefManager {
19972017
// Look in all the proper paths, in this order
19982018
const allpaths = refpaths(ref);
19992019
for (const ref of allpaths) {
2000-
if (await fs.exists(`${gitdir}/${ref}`)) return ref
2020+
const refExists = await acquireLock(ref, async () =>
2021+
fs.exists(`${gitdir}/${ref}`)
2022+
);
2023+
if (refExists) return ref
20012024
if (packedMap.has(ref)) return ref
20022025
}
20032026
// Do we give up?
@@ -2048,7 +2071,9 @@ class GitRefManager {
20482071
}
20492072

20502073
static async packedRefs({ fs, gitdir }) {
2051-
const text = await fs.read(`${gitdir}/packed-refs`, { encoding: 'utf8' });
2074+
const text = await acquireLock('packed-refs', async () =>
2075+
fs.read(`${gitdir}/packed-refs`, { encoding: 'utf8' })
2076+
);
20522077
const packed = GitPackedRefs.from(text);
20532078
return packed.refs
20542079
}
@@ -7210,14 +7235,14 @@ class GitRemoteManager {
72107235
}
72117236
}
72127237

7213-
let lock$1 = null;
7238+
let lock$2 = null;
72147239

72157240
class GitShallowManager {
72167241
static async read({ fs, gitdir }) {
7217-
if (lock$1 === null) lock$1 = new AsyncLock();
7242+
if (lock$2 === null) lock$2 = new AsyncLock();
72187243
const filepath = join(gitdir, 'shallow');
72197244
const oids = new Set();
7220-
await lock$1.acquire(filepath, async function() {
7245+
await lock$2.acquire(filepath, async function() {
72217246
const text = await fs.read(filepath, { encoding: 'utf8' });
72227247
if (text === null) return oids // no file
72237248
if (text.trim() === '') return oids // empty file
@@ -7230,18 +7255,18 @@ class GitShallowManager {
72307255
}
72317256

72327257
static async write({ fs, gitdir, oids }) {
7233-
if (lock$1 === null) lock$1 = new AsyncLock();
7258+
if (lock$2 === null) lock$2 = new AsyncLock();
72347259
const filepath = join(gitdir, 'shallow');
72357260
if (oids.size > 0) {
72367261
const text = [...oids].join('\n') + '\n';
7237-
await lock$1.acquire(filepath, async function() {
7262+
await lock$2.acquire(filepath, async function() {
72387263
await fs.write(filepath, text, {
72397264
encoding: 'utf8',
72407265
});
72417266
});
72427267
} else {
72437268
// No shallows
7244-
await lock$1.acquire(filepath, async function() {
7269+
await lock$2.acquire(filepath, async function() {
72457270
await fs.rm(filepath);
72467271
});
72477272
}

0 commit comments

Comments
 (0)