Skip to content

Replace racey mutexed map with skip coordinator#97

Merged
MusicalNinjaDad merged 4 commits into
mainfrom
race
Nov 6, 2025
Merged

Replace racey mutexed map with skip coordinator#97
MusicalNinjaDad merged 4 commits into
mainfrom
race

Conversation

@MusicalNinjaDad

@MusicalNinjaDad MusicalNinjaDad commented Nov 6, 2025 •
edited by sourcery-ai Bot
Loading

Copy link
Copy Markdown
Owner

closes #96

Summary by Sourcery

Replace mutex-based file locking with a channel-driven skip coordinator to dedupe duplicate link operations and update related logic

Enhancements:

  • Replace fileLocks sync.Mutex map with a skipCheck channel and skipHandler to coordinate and skip redundant link/copy actions
  • Refactor link and snaggle functions to send skipCheck requests and conditionally perform link, copy, or skip operations
  • Remove fileLocks type and sync import, replacing them with skipCheck and hardLink types and the skipHandler goroutine

Tests:

  • Update stdout assertions to recognize ‘skip’ prefixes and ensure each file is linked or copied only once

@sourcery-ai

sourcery-ai Bot commented Nov 6, 2025 •
edited
Loading

Copy link
Copy Markdown

Reviewer's Guide

The PR removes the race-prone mutex-based fileLocks in favor of a channel-driven skip coordinator to serialize duplicate hard-link operations, updates the link/snwaggle functions to use this new coordinator, and adjusts tests to validate the new skip behavior.

Class diagram for skip coordinator and related types

classDiagram
    class skipCheck {
        +hardLink hardLink
        +chan bool response
    }
    class hardLink {
        +string source
        +string resolved
        +string destination
    }
    class skipHandler {
    }
    skipHandler --|> skipCheck
    skipCheck o-- hardLink
Loading

Flow diagram for skipHandler logic

flowchart TD
    A["Receive skipCheck request"] --> B{"destination already linked?"}
    B -- No --> C["Mark destination as linked"]
    C --> D["Respond: false (do not skip)"]
    B -- Yes --> E["Respond: true (skip)"]
Loading

File-Level Changes

Change Details Files
Replace sync-based fileLocks with channel-driven skip coordinator
  • Removed fileLocks struct, newFileLock(), and lock/unlock methods
  • Removed sync import
  • Added skipCheck and hardLink structs
  • Implemented skipHandler function
  • Initialized checker channel and launched skipHandler in Snaggle
 snaggle.go
Integrate skip coordinator into link & snaggle workflows
  • Changed link and snaggle signatures to accept checker channel
  • In link(), replaced locks.lock/unlock with skipCheck send/receive and skip branch
  • Updated all link() call sites to pass the checker channel
 snaggle.go
Extend test assertions to validate skip semantics
  • Allowed 'skip' prefix in Stdout assertions
  • Ensured duplicate files trigger skip rather than link/copy
  • Updated error messages and prefix checks for unexpected lines
 internal/testing/assertions.go

Assessment against linked issues

Issue Objective Addressed Explanation
#96 Replace the use of a map (with mutex) for link coordination with a channel-based coordinator to ensure thread safety and improve performance. ✅
#96 Ensure that concurrent read-write operations for link coordination are safe and do not use unsafe map access. ✅
#96 Update code and tests to reflect the new channel-based coordinator, including correct reporting of skipped links. ✅
Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@codecov

codecov Bot commented Nov 6, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.80488% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.42%. Comparing base (7ca3e57) to head (0f1d9bd).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/testing/assertions.go 37.50% 3 Missing and 2 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main      #97      +/-   ##
==========================================
- Coverage   74.67%   74.42%   -0.25%     
==========================================
  Files          12       12              
  Lines         687      696       +9     
==========================================
+ Hits          513      518       +5     
- Misses        120      122       +2     
- Partials       54       56       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sourcery-ai[bot]
sourcery-ai Bot reviewed Nov 6, 2025
View reviewed changes

@sourcery-ai sourcery-ai Bot 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.

Hey there - I've reviewed your changes and they look great!

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@MusicalNinjaDad

MusicalNinjaDad commented Nov 6, 2025

Copy link
Copy Markdown
Owner Author

@sourcery-ai summary
@sourcery-ai guide
@sourcery-ai review

@MusicalNinjaDad MusicalNinjaDad merged commit 3a99d54 into main Nov 6, 2025
16 checks passed
@MusicalNinjaDad MusicalNinjaDad deleted the race branch November 6, 2025 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sourcery-ai sourcery-ai[bot] sourcery-ai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Use a channel-based link coordinator, map is unsafe for concurrent read-write and v. slow if adding a Rlock

1 participant

@MusicalNinjaDad