From 3b7431b6ac27f8557c22a8959ae1ce431f6d2167 Mon Sep 17 00:00:00 2001
From: Romain Marcadier <rmuller@amazon.fr>
Date: Mon, 20 Mar 2023 18:21:09 +0100
Subject: [PATCH] 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*
---
 packages/aws-cdk/lib/api/util/rwlock.ts | 26 ++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/packages/aws-cdk/lib/api/util/rwlock.ts b/packages/aws-cdk/lib/api/util/rwlock.ts
index 6667bd17afe06..6fb0e4c945dfd 100644
--- a/packages/aws-cdk/lib/api/util/rwlock.ts
+++ b/packages/aws-cdk/lib/api/util/rwlock.ts
@@ -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`);
   }
 
   /**
@@ -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);
       },
     };
   }
@@ -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;
@@ -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);
 }
@@ -181,4 +193,4 @@ function processExists(pid: number) {
   } catch (e) {
     return false;
   }
-}
\ No newline at end of file
+}