Skip to content

Fix pointer increment issue in _md_shrink causing memory corruption#1222

Merged
bdraco merged 4 commits intoaio-libs:masterfrom
bdraco:shrink_fix
Jun 28, 2025
Merged

Fix pointer increment issue in _md_shrink causing memory corruption#1222
bdraco merged 4 commits intoaio-libs:masterfrom
bdraco:shrink_fix

Conversation

@bdraco
Copy link
Member

@bdraco bdraco commented Jun 28, 2025

What do these changes do?

This PR addresses a subtle pointer arithmetic issue in the _md_shrink function that was part of the memory optimization improvements in PR #1200 (version 6.6.0).

The _md_shrink function optimizes memory usage by compacting the internal hash table in-place when there are deleted entries. However, there was an edge case where the pointer increment logic didn't account for when new_ep == old_ep (which occurs for the first non-deleted entry after deletions).

This PR adjusts the pointer increment to happen for all non-deleted entries, ensuring the compaction process maintains data integrity throughout the operation.

Are there changes in behavior for the user?

No API or functionality changes. This fix ensures the memory optimization continues to work as intended while maintaining data integrity. Users who experienced stability issues with 6.6.0 should see those resolved.

Related issue number

Fixes #1221

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Jun 28, 2025
@bdraco bdraco changed the title fix shrink corruption Fix pointer increment issue in _md_shrink causing memory corruption Jun 28, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Jun 28, 2025

CodSpeed Performance Report

Merging #1222 will not alter performance

Comparing bdraco:shrink_fix (1ff2e2b) with master (863eff1)

Summary

✅ 245 untouched benchmarks

@codecov
Copy link

codecov bot commented Jun 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.39%. Comparing base (863eff1) to head (1ff2e2b).
⚠️ Report is 13 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1222   +/-   ##
=======================================
  Coverage   98.38%   98.39%           
=======================================
  Files          27       27           
  Lines        3973     3988   +15     
  Branches      730      735    +5     
=======================================
+ Hits         3909     3924   +15     
  Misses         17       17           
  Partials       47       47           
Flag Coverage Δ
CI-GHA 98.39% <100.00%> (+<0.01%) ⬆️
MyPy 80.51% <100.00%> (+0.07%) ⬆️
pytest 99.85% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Member

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

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

Looks good, thank you for the quick PR.

As you see, @webknjaz and I decided that even people with write access shouldn't push new branches in this repo but create forks instead.

The release process is still pretty similar: merge a PR with version bump and towncrier build applied, then switch to master, pull the latest changes, make a tag and push it.

If you have any problems, I could do release myself in the evening.

@bdraco
Copy link
Member Author

bdraco commented Jun 28, 2025

I'll yank 6.6.0 and 6.6.1 and do a release. Will ping if I have any issues

@bdraco bdraco merged commit 17bbfd0 into aio-libs:master Jun 28, 2025
64 checks passed
@bdraco bdraco deleted the shrink_fix branch June 28, 2025 12:29
@asvetlov
Copy link
Member

Pushing tags in the upstream (aio-libs:multidict) is allowed, only new branches and direct commits to the master are forbidden.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6.6.0 or 6.6.1 has introduced a segfault

2 participants