Skip to content

fix(logger): prevent concurrent map read/write in ReduceFunctionName - #13

Merged
simonnix merged 1 commit into
mainfrom
fix/logger-concurrent-map-access
Nov 2, 2026
Merged

simonnix merged 1 commit into
mainfrom
fix/logger-concurrent-map-access

Conversation

@simonnix

@simonnix simonnix commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Acquire functionLock before checking functionCache in ReduceFunctionName to prevent data races when multiple goroutines log simultaneously.

Summary by CodeRabbit

  • Bug Fixes
    • Improved reliability when function names are processed concurrently, preventing potential inconsistencies in the results.

Acquire functionLock before checking functionCache in ReduceFunctionName
to prevent data races when multiple goroutines log simultaneously.
@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a8e2d46a-e0ae-4131-ad09-aa15719b5a73
Commits

Reviewing files that changed from the base of the PR and between 5e8097d and 6aad421.

Files selected for processing: 1
  • internal/logger/reduce.go

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Walkthrough

ReduceFunctionName now acquires functionLock before checking functionCache. The deferred unlock continues to cover the function body.

Changes: Function name cache locking

Layer / File(s) Summary
Lock before cache lookup
internal/logger/reduce.go
ReduceFunctionName acquires functionLock before reading functionCache. The deferred unlock remains in place.

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Merge Risk: Minimal · up to 6aad4

The cache race fix is sound and introduces no identified merge-blocking risk.

Architecture risk: Low · up to 6aad4

The change affects 1 system.

Changed systems: internal

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — internal (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in internal/logger/reduce.go: ReduceFunctionName now locks before reading functionCache; previously the cache lookup occurred before locking. The deferred unlock still covers the remaining function body.
Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: preventing concurrent map read/write in ReduceFunctionName.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@codecov

codecov Bot commented Nov 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@simonnix
simonnix merged commit b575b37 into main Nov 2, 2026
4 checks passed
@simonnix
simonnix deleted the fix/logger-concurrent-map-access branch November 2, 2026 12:45
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.

1 participant