Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(toolkit): RWLock.acquireRead is not re-entrant #24702

Merged
merged 2 commits into from
Mar 20, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions packages/aws-cdk/lib/api/util/rwlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,12 @@ import * as path from 'path';
export class RWLock {
private readonly pidString: string;
private readonly writerFile: string;
private readonly readerFile: string;
private readCounter = 0;

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

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

/**
Expand Down Expand Up @@ -62,14 +61,26 @@ export class RWLock {
return this.doAcquireRead();
}

/**
* Obtains the name fo a (new) `readerFile` to use. This includes a counter so
* that if multiple threads of the same PID attempt to concurrently acquire
* the same lock, they're guaranteed to use a different reader file name (only
* one thread will ever execute JS code at once, guaranteeing the readCounter
* is incremented "atomically" from the point of view of this PID.).
*/
private readerFile(): string {
return path.join(this.directory, `read.${this.pidString}.${++this.readCounter}.lock`);
}

/**
* Do the actual acquiring of a read lock.
*/
private async doAcquireRead(): Promise<ILock> {
await writeFileAtomic(this.readerFile, this.pidString);
const readerFile = this.readerFile();
await writeFileAtomic(readerFile, this.pidString);
return {
release: async () => {
await deleteFile(this.readerFile);
await deleteFile(readerFile);
},
};
}
Expand Down Expand Up @@ -102,7 +113,7 @@ export class RWLock {
* Check the current readers (if any)
*/
private async currentReaders(): Promise<number[]> {
const re = /^read\.([^.]+)\.lock$/;
const re = /^read\.([^.]+)\.[^.]+\.lock$/;
const ret = new Array<number>();

let children;
Expand Down Expand Up @@ -156,9 +167,10 @@ async function readFileIfExists(filename: string): Promise<string | undefined> {
}
}

let tmpCounter = 0;
async function writeFileAtomic(filename: string, contents: string): Promise<void> {
await fs.mkdir(path.dirname(filename), { recursive: true });
const tmpFile = `${filename}.${process.pid}`;
const tmpFile = `${filename}.${process.pid}_${++tmpCounter}`;
await fs.writeFile(tmpFile, contents, { encoding: 'utf-8' });
await fs.rename(tmpFile, filename);
}
Expand All @@ -181,4 +193,4 @@ function processExists(pid: number) {
} catch (e) {
return false;
}
}
}