Skip to content

Commit 3b7431b

Browse files
authored
fix(toolkit): RWLock.acquireRead is not re-entrant (#24702)
If multiple threads of the same process attempt to acquire the same reader lock, the a race condition occurs, and the first thread to release the reader lock will release ALL the locks. Introduce a counter so that each acquire attempt uses a different file name, ensuring that the read lock is reentrant. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 9744a82 commit 3b7431b

File tree

1 file changed

+19
-7
lines changed

1 file changed

+19
-7
lines changed

packages/aws-cdk/lib/api/util/rwlock.ts

+19-7
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,12 @@ import * as path from 'path';
1414
export class RWLock {
1515
private readonly pidString: string;
1616
private readonly writerFile: string;
17-
private readonly readerFile: string;
17+
private readCounter = 0;
1818

1919
constructor(public readonly directory: string) {
2020
this.pidString = `${process.pid}`;
2121

2222
this.writerFile = path.join(this.directory, 'synth.lock');
23-
this.readerFile = path.join(this.directory, `read.${this.pidString}.lock`);
2423
}
2524

2625
/**
@@ -62,14 +61,26 @@ export class RWLock {
6261
return this.doAcquireRead();
6362
}
6463

64+
/**
65+
* Obtains the name fo a (new) `readerFile` to use. This includes a counter so
66+
* that if multiple threads of the same PID attempt to concurrently acquire
67+
* the same lock, they're guaranteed to use a different reader file name (only
68+
* one thread will ever execute JS code at once, guaranteeing the readCounter
69+
* is incremented "atomically" from the point of view of this PID.).
70+
*/
71+
private readerFile(): string {
72+
return path.join(this.directory, `read.${this.pidString}.${++this.readCounter}.lock`);
73+
}
74+
6575
/**
6676
* Do the actual acquiring of a read lock.
6777
*/
6878
private async doAcquireRead(): Promise<ILock> {
69-
await writeFileAtomic(this.readerFile, this.pidString);
79+
const readerFile = this.readerFile();
80+
await writeFileAtomic(readerFile, this.pidString);
7081
return {
7182
release: async () => {
72-
await deleteFile(this.readerFile);
83+
await deleteFile(readerFile);
7384
},
7485
};
7586
}
@@ -102,7 +113,7 @@ export class RWLock {
102113
* Check the current readers (if any)
103114
*/
104115
private async currentReaders(): Promise<number[]> {
105-
const re = /^read\.([^.]+)\.lock$/;
116+
const re = /^read\.([^.]+)\.[^.]+\.lock$/;
106117
const ret = new Array<number>();
107118

108119
let children;
@@ -156,9 +167,10 @@ async function readFileIfExists(filename: string): Promise<string | undefined> {
156167
}
157168
}
158169

170+
let tmpCounter = 0;
159171
async function writeFileAtomic(filename: string, contents: string): Promise<void> {
160172
await fs.mkdir(path.dirname(filename), { recursive: true });
161-
const tmpFile = `${filename}.${process.pid}`;
173+
const tmpFile = `${filename}.${process.pid}_${++tmpCounter}`;
162174
await fs.writeFile(tmpFile, contents, { encoding: 'utf-8' });
163175
await fs.rename(tmpFile, filename);
164176
}
@@ -181,4 +193,4 @@ function processExists(pid: number) {
181193
} catch (e) {
182194
return false;
183195
}
184-
}
196+
}

0 commit comments

Comments
 (0)