-
Notifications
You must be signed in to change notification settings - Fork 1
Fix race conditions in mkdocs.yml file watcher callback #59
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
365316e
fix: prevent race conditions in mkdocs.yml file watcher callback
jcouball 031b234
fix: address Copilot review comments (round 2)
jcouball e0a5ac7
fix: address Copilot review comments (round 3)
jcouball 4663a14
fix: address Copilot review comments (round 4)
jcouball a70a083
fix: address Copilot review comments (round 5)
jcouball File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| /** | ||
| * AsyncSerializer ensures that async operations are executed serially, | ||
| * preventing race conditions when the same operation is triggered multiple times rapidly. | ||
| * | ||
| * When an operation is already in progress, subsequent calls are coalesced - the operation | ||
| * will run again after the current execution completes, but only once regardless of how many | ||
| * times execute() was called during that time. | ||
| * | ||
| * Example usage: | ||
| * ```typescript | ||
| * const serializer = new AsyncSerializer(); | ||
| * | ||
| * // These rapid calls won't overlap - first runs immediately, | ||
| * // subsequent calls are coalesced into one re-execution | ||
| * await serializer.execute(async () => { | ||
| * await loadConfig(); | ||
| * refreshUI(); | ||
| * }); | ||
| * ``` | ||
| */ | ||
| export class AsyncSerializer { | ||
| private _isExecuting = false; | ||
| private _pendingFn: (() => Promise<void>) | undefined = undefined; | ||
|
|
||
| /** | ||
| * Execute an async operation, ensuring it doesn't overlap with previous calls. | ||
| * | ||
| * If an execution is already in progress, this call stores {@link fn} as the next | ||
| * operation to run and returns immediately. When the current execution finishes, the | ||
| * most recently stored pending function will be executed. Multiple calls during | ||
| * execution are coalesced — only the last-queued function runs for the coalesced | ||
| * re-execution. | ||
| * | ||
| * @param fn The async operation to execute | ||
| * @returns A promise whose meaning depends on whether this call starts a new | ||
| * execution cycle. If no execution is in progress, the promise resolves | ||
| * after the current execution cycle completes, including any coalesced | ||
| * re-execution triggered while it is running, and rejects if the final | ||
| * execution of that cycle fails. If an execution is already in progress, | ||
| * the promise resolves once this call has been acknowledged and {@link fn} | ||
| * has been stored as the latest pending operation; in that case, the | ||
| * returned promise does not observe the eventual success or failure of the | ||
| * running/coalesced execution and does not guarantee a distinct execution | ||
| * of {@link fn}. | ||
| */ | ||
| async execute(fn: () => Promise<void>): Promise<void> { | ||
| // If already executing, store latest fn for the coalesced re-run and return immediately | ||
| if (this._isExecuting) { | ||
| this._pendingFn = fn; | ||
| return; | ||
| } | ||
|
|
||
| this._isExecuting = true; | ||
| let lastError: unknown | undefined; | ||
| try { | ||
| // Keep executing while new requests come in, always using the latest queued fn | ||
| let nextFn: (() => Promise<void>) | undefined = fn; | ||
| while (nextFn !== undefined) { | ||
| const toRun = nextFn; | ||
| this._pendingFn = undefined; | ||
| try { | ||
| await toRun(); | ||
| lastError = undefined; // Clear error if a subsequent execution succeeds | ||
| } catch (error) { | ||
| lastError = error; | ||
| } | ||
| nextFn = this._pendingFn; | ||
| } | ||
| } finally { | ||
| this._isExecuting = false; | ||
| this._pendingFn = undefined; | ||
| } | ||
|
|
||
| if (lastError !== undefined) { | ||
| throw lastError; | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,172 @@ | ||
| import * as assert from 'assert'; | ||
| import { AsyncSerializer } from '../../asyncSerializer'; | ||
|
|
||
| /** | ||
| * Unit tests for AsyncSerializer | ||
| * | ||
| * AsyncSerializer ensures that async operations are executed serially, | ||
| * preventing race conditions when the same async operation is triggered | ||
| * multiple times rapidly. | ||
| */ | ||
| describe('AsyncSerializer', () => { | ||
| it('should prevent concurrent executions', async () => { | ||
| const serializer = new AsyncSerializer(); | ||
| let inFlight = 0; | ||
| let maxInFlight = 0; | ||
|
|
||
| // A manually controlled barrier holds the operation in-flight until released, | ||
| // making this test deterministic and free of real-time delays. | ||
| let release!: () => void; | ||
| const barrier = new Promise<void>(resolve => { release = resolve; }); | ||
|
|
||
| const operation = async () => { | ||
| inFlight++; | ||
| if (inFlight > maxInFlight) { maxInFlight = inFlight; } | ||
| await barrier; | ||
| inFlight--; | ||
| }; | ||
|
|
||
| // Fire three operations; first executes immediately, the rest are coalesced | ||
| const promises = [ | ||
| serializer.execute(operation), | ||
| serializer.execute(operation), | ||
| serializer.execute(operation), | ||
| ]; | ||
|
|
||
| // Release the barrier so all executions can complete | ||
| release(); | ||
| await Promise.all(promises); | ||
|
|
||
| assert.strictEqual(maxInFlight, 1, 'At most one execution should be in flight at a time'); | ||
| }); | ||
|
|
||
| it('should coalesce multiple pending calls into a single additional execution', async () => { | ||
| const serializer = new AsyncSerializer(); | ||
| let executionCount = 0; | ||
|
|
||
| // A manually controlled barrier holds the first execution in-flight while | ||
| // the remaining calls are dispatched synchronously, ensuring coalescing occurs. | ||
| let release!: () => void; | ||
| const barrier = new Promise<void>(resolve => { release = resolve; }); | ||
|
|
||
| const countingOperation = async () => { | ||
| executionCount++; | ||
| await barrier; | ||
| }; | ||
|
|
||
| // Fire five operations rapidly while first is running | ||
| const promises = [ | ||
| serializer.execute(countingOperation), | ||
| serializer.execute(countingOperation), | ||
| serializer.execute(countingOperation), | ||
| serializer.execute(countingOperation), | ||
| serializer.execute(countingOperation), | ||
| ]; | ||
|
|
||
| release(); | ||
| await Promise.all(promises); | ||
|
|
||
| // With coalescing of synchronously scheduled calls, this should be deterministic: | ||
| // - First call runs immediately. | ||
| // - Calls 2-5 are all queued while the first is running and are coalesced into | ||
| // a single additional execution that services all pending callers. | ||
| // Therefore we expect exactly 2 executions. | ||
| assert.strictEqual( | ||
| executionCount, | ||
| 2, | ||
| `Expected exactly 2 executions with coalescing, got ${executionCount}` | ||
| ); | ||
| }); | ||
|
|
||
| it('should execute immediately if not already running', async () => { | ||
| const serializer = new AsyncSerializer(); | ||
| let executed = false; | ||
|
|
||
| await serializer.execute(async () => { | ||
| executed = true; | ||
| }); | ||
|
|
||
| assert.strictEqual(executed, true, 'Should execute immediately when not busy'); | ||
| }); | ||
|
|
||
| it('should handle errors without breaking serialization', async () => { | ||
| const serializer = new AsyncSerializer(); | ||
| const executions: number[] = []; | ||
|
|
||
| const failingOperation = async () => { | ||
| executions.push(1); | ||
| throw new Error('Operation failed'); | ||
| }; | ||
|
|
||
| const successOperation = async () => { | ||
| executions.push(2); | ||
| }; | ||
|
|
||
| // First operation fails | ||
| await serializer.execute(failingOperation).catch(() => {}); | ||
|
|
||
| // Second operation should still run | ||
| await serializer.execute(successOperation); | ||
|
|
||
| assert.deepStrictEqual(executions, [1, 2], 'Should execute both operations despite error'); | ||
| }); | ||
|
jcouball marked this conversation as resolved.
|
||
|
|
||
| it('should execute coalesced pending calls even when an error occurs', async () => { | ||
| const serializer = new AsyncSerializer(); | ||
| let executionCount = 0; | ||
|
|
||
| // A manually controlled barrier holds the first execution in-flight so calls | ||
| // B and C are coalesced before A finishes (and throws). | ||
| let release!: () => void; | ||
| const barrier = new Promise<void>(resolve => { release = resolve; }); | ||
|
|
||
| const operation = async () => { | ||
| const id = ++executionCount; | ||
| await barrier; | ||
| if (id === 1) { | ||
| throw new Error('First execution failed'); | ||
| } | ||
| }; | ||
|
|
||
| // Start call A (will error partway through but coalesced re-run succeeds), | ||
| // then dispatch B and C which get coalesced | ||
| const promiseA = serializer.execute(operation); | ||
| const promiseB = serializer.execute(operation); | ||
| const promiseC = serializer.execute(operation); | ||
|
|
||
| release(); | ||
|
|
||
| // promiseA resolves (not rejects) because the coalesced re-run succeeded, | ||
| // clearing the error from the first execution | ||
| await promiseA; | ||
| await promiseB; | ||
| await promiseC; | ||
|
|
||
| // The pending coalesced work must still have been executed despite A's error | ||
| assert.strictEqual( | ||
| executionCount, | ||
| 2, | ||
| `Expected 2 executions (A + coalesced B/C), got ${executionCount}` | ||
| ); | ||
| }); | ||
|
|
||
| it('should handle rapid sequential calls after completion', async () => { | ||
| const serializer = new AsyncSerializer(); | ||
| const executions: number[] = []; | ||
|
|
||
| const operation = async (id: number) => { | ||
| executions.push(id); | ||
| await Promise.resolve(); | ||
| }; | ||
|
|
||
| // First batch | ||
| await serializer.execute(() => operation(1)); | ||
| await serializer.execute(() => operation(2)); | ||
|
|
||
| // Second batch after first completes | ||
| await serializer.execute(() => operation(3)); | ||
| await serializer.execute(() => operation(4)); | ||
|
|
||
| assert.deepStrictEqual(executions, [1, 2, 3, 4], 'Should execute all operations in order'); | ||
| }); | ||
| }); | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.