Skip to content

Add mutex on LinkFromPool to fix #1449#1481

Merged
neolynx merged 1 commit into
aptly-dev:masterfrom
agustinhenze:master
Aug 29, 2025
Merged

Add mutex on LinkFromPool to fix #1449#1481
neolynx merged 1 commit into
aptly-dev:masterfrom
agustinhenze:master

Conversation

@agustinhenze

@agustinhenze agustinhenze commented Aug 19, 2025

Copy link
Copy Markdown
Contributor

This fixes the race condition that happens when you call publish concurrently. It adds a valuable test that reproduces the error almost deterministically, it's hard to say always but I have run this in loop 100 times and it reproduces the error consistently without the patch and after the patch it works consistently.

Fixes #1449

Checklist

  • unit-test added (To reproduce the race condition)
  • author name in AUTHORS

@codecov

codecov Bot commented Aug 19, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.55%. Comparing base (8ca4cb8) to head (5f464bd).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1481      +/-   ##
==========================================
- Coverage   74.86%   74.55%   -0.31%     
==========================================
  Files         159      159              
  Lines       18438    18454      +16     
==========================================
- Hits        13803    13759      -44     
- Misses       3489     3553      +64     
+ Partials     1146     1142       -4     

☔ 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.

@neolynx neolynx self-assigned this Aug 27, 2025
@neolynx neolynx requested a review from a team August 27, 2025 21:26
@neolynx

neolynx commented Aug 27, 2025

Copy link
Copy Markdown
Member

you guys are heroes ! 🥇

@iofq iofq left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. One nitpick about the length of the unit test, but fine to merge as-is.

Comment thread files/linkfrompool_concurrency_test.go Outdated
This fixes the race condition that happens when you call publish
concurrently. It adds a valuable test that reproduces the error almost
deterministically, it's hard to say always but I have run this in loop
100 times and it reproduces the error consistently without the patch and
after the patch it works consistently.
@iofq

iofq commented Aug 29, 2025

Copy link
Copy Markdown
Contributor

@neolynx can you merge? :)

@neolynx neolynx merged commit d3b7186 into aptly-dev:master Aug 29, 2025
37 checks passed
@neolynx neolynx mentioned this pull request Jun 11, 2026
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.

packages missing after concurrent publish

3 participants