Skip to content

Fix race conditions in mkdocs.yml file watcher callback#59

Merged
jcouball merged 5 commits into
mainfrom
fix/race-condition-mkdocs-watcher
Apr 24, 2026
Merged

Fix race conditions in mkdocs.yml file watcher callback#59
jcouball merged 5 commits into
mainfrom
fix/race-condition-mkdocs-watcher

Conversation

@jcouball

Copy link
Copy Markdown
Member

Description

Fixes #48 - Prevents race conditions when mkdocs.yml is saved multiple times rapidly.

Problem

The reloadMkdocsConfig callback in extension.ts could be called multiple times rapidly if mkdocs.yml was saved multiple times quickly (e.g., during auto-save or rapid edits). Since loadMkdocsConfig() is async, this created potential race conditions:

  1. Multiple config loads could be in-flight simultaneously
  2. The forEach loop that refreshes diagnostics could operate on stale config data if a newer config load completes after the loop starts
  3. Diagnostics could briefly show incorrect severity due to timing issues

Solution

Implemented an AsyncSerializer class that uses a semaphore pattern to ensure only one config reload happens at a time, with coalescing of rapid events.

Key Features

  • Prevents concurrent executions: Only one async operation runs at a time
  • Coalesces rapid calls: Multiple calls during execution result in just one re-execution
  • Guarantees consistency: The most recent config is always loaded with diagnostics refreshed using that config
  • Error resilient: Handles errors without breaking the serialization

Implementation Details

New Files

  • src/asyncSerializer.ts - Pure business logic for serializing async operations
  • src/test/unit/asyncSerializer.test.ts - Comprehensive unit tests with timing instrumentation

Modified Files

  • src/extension.ts - Integrated AsyncSerializer into reloadMkdocsConfig callback

TDD Approach

This fix was developed using strict Test-Driven Development:

  1. RED: Created failing unit tests that verify no concurrent executions occur
  2. GREEN: Implemented AsyncSerializer with do-while semaphore pattern
  3. REFACTOR: Integrated into extension.ts

Testing

  • ✅ 5 new unit tests for AsyncSerializer
  • ✅ All 134 tests passing (124 unit + 10 integration)
  • ✅ 100% code coverage maintained
  • ✅ Tests verify:
    • No concurrent executions occur
    • Multiple pending calls are coalesced
    • Immediate execution when not busy
    • Error handling doesn't break serialization
    • Sequential calls after completion work correctly

Performance Impact

  • Positive: Eliminates redundant config reloads when file changes rapidly
  • Negligible overhead: Simple boolean flags and do-while loop
  • No blocking: Returns immediately if already executing

Related

Copilot AI review requested due to automatic review settings December 19, 2025 01:02

This comment was marked as resolved.

jcouball added a commit that referenced this pull request Apr 24, 2026
- Fix error handling in AsyncSerializer do-while loop so coalesced
  pending calls still execute when fn() throws an error
- Fix misleading JSDoc @returns to clarify coalesced calls share
  execution cycles
- Tighten coalescing test assertion from '2-5' to exactly 2
- Add test verifying coalesced pending calls execute despite errors
Add AsyncSerializer class to serialize rapid mkdocs.yml config reloads,
preventing race conditions when the file is saved multiple times quickly.

The new AsyncSerializer:
- Prevents concurrent execution of async operations
- Coalesces multiple rapid calls into single re-execution
- Ensures the most recent config is loaded
- Guarantees diagnostics are refreshed with consistent data

Following TDD methodology:
1. Created comprehensive unit tests with timing instrumentation
2. Implemented AsyncSerializer with do-while semaphore pattern
3. Integrated into extension.ts reloadMkdocsConfig callback
4. All tests pass with 100% coverage maintained

Resolves #48
@jcouball
jcouball force-pushed the fix/race-condition-mkdocs-watcher branch from 9b2de37 to 365316e Compare April 24, 2026 22:46
@jcouball
jcouball requested a review from Copilot April 24, 2026 22:47

This comment was marked as resolved.

- Rename private fields to use underscore prefix per project conventions
  (_isExecuting, _pendingFn)
- Replace hasPending boolean with _pendingFn storage so coalesced
  re-runs always execute the latest queued function, not the original

This comment was marked as resolved.

- Rewrite timing-dependent AsyncSerializer tests to use deterministic
  deferred promise barriers and inFlight counters instead of
  setTimeout/Date.now, eliminating flakiness under CI load
- Fix reloadMkdocsConfig to read vscode.workspace.workspaceFolders
  live inside the callback rather than using the stale captured
  variable, so multi-root workspaces and folder changes are handled
  correctly

This comment was marked as resolved.

- Update AsyncSerializer JSDoc to document rejection behavior: the
  returned promise can reject, rethrowing the first error after all
  pending executions have been processed
- Wrap reloadMkdocsConfig body in try/catch to prevent unhandled
  promise rejections from propagating to the extension host; errors
  are logged to the output channel and shown as a user-friendly message
- Rename coalescing test to 'should coalesce multiple pending calls
  into a single additional execution' for accuracy

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread src/asyncSerializer.ts Outdated
Comment thread src/asyncSerializer.ts Outdated
- Track last error instead of first error in AsyncSerializer: clear the
  error after each successful execution so the cycle only rejects if
  the final execution fails. A later coalesced success no longer causes
  a spurious rejection.
- Update @returns JSDoc to precisely document the two-contract behavior:
  cycle-starters observe resolve/reject of the full cycle; coalesced
  callers only observe acknowledgement and do not see success/failure.
- Update the coalesced-error test to reflect that promiseA now resolves
  (not rejects) when the coalesced re-run succeeds.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@jcouball
jcouball merged commit f0bc616 into main Apr 24, 2026
12 checks passed
@jcouball
jcouball deleted the fix/race-condition-mkdocs-watcher branch April 24, 2026 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prevent race conditions in mkdocs.yml file watcher callback

2 participants