Skip to content

Fix segfault in PyType_GetModuleByDef for Python < 3.11#1141

Merged
bdraco merged 11 commits intomasterfrom
fix_segfault_PyType_GetModuleByDef
Apr 9, 2025
Merged

Fix segfault in PyType_GetModuleByDef for Python < 3.11#1141
bdraco merged 11 commits intomasterfrom
fix_segfault_PyType_GetModuleByDef

Conversation

@bdraco
Copy link
Member

@bdraco bdraco commented Apr 9, 2025

fixes #1140

bdraco
bdraco commented Apr 9, 2025
View reviewed changes
asvetlov
asvetlov reviewed Apr 9, 2025
View reviewed changes
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.

It seems that the fix is correct.

We need a test for this case to never get the bug again.
@bdraco could you prepare it? A simple case with an inheritance from MiltoDict should be enough I guess

asvetlov
asvetlov reviewed Apr 9, 2025
View reviewed changes
bdraco and others added 4 commits April 9, 2025 11:42
@bdraco
changelog
994b7ac
@bdraco @asvetlov
Update multidict/_multilib/state.h
42ada0f 
Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
@bdraco
Merge remote-tracking branch 'origin/fix_segfault_PyType_GetModuleByD…
aff405c 
…ef' into fix_segfault_PyType_GetModuleByDef
@bdraco
preen
28e0e10
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Apr 9, 2025
@bdraco
Copy link
Member Author

bdraco commented Apr 9, 2025

verified test segfaults before the fix

@bdraco bdraco marked this pull request as ready for review April 9, 2025 21:46
@bdraco bdraco requested a review from webknjaz as a code owner April 9, 2025 21:46
@bdraco
Copy link
Member Author

bdraco commented Apr 9, 2025 •
edited
Loading

Thanks for the quick look, if everything passes, I’ll merge and get another patch release done

@asvetlov
Copy link
Member

asvetlov commented Apr 9, 2025

Thank you for the quick patch!

@codspeed-hq
Copy link

codspeed-hq bot commented Apr 9, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #1141 will not alter performance

Comparing fix_segfault_PyType_GetModuleByDef (b66d70e) with master (2c0d458)

Summary

✅ 244 untouched benchmarks

@bdraco bdraco enabled auto-merge (squash) April 9, 2025 21:57
@bdraco
Copy link
Member Author

bdraco commented Apr 9, 2025

Github checks are out of sync. Will close reopen to rerun

@bdraco bdraco closed this Apr 9, 2025
auto-merge was automatically disabled April 9, 2025 21:57

Pull request was closed

@bdraco bdraco reopened this Apr 9, 2025
@bdraco bdraco enabled auto-merge (squash) April 9, 2025 21:58
@bdraco bdraco merged commit 21a6e93 into master Apr 9, 2025
45 checks passed
@codecov
Copy link

codecov bot commented Apr 9, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.69%. Comparing base (2c0d458) to head (b66d70e).
Report is 85 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #1141   +/-   ##
=======================================
  Coverage   98.68%   98.69%           
=======================================
  Files          27       27           
  Lines        3432     3441    +9     
  Branches      528      528           
=======================================
+ Hits         3387     3396    +9     
  Misses         17       17           
  Partials       28       28
Flag Coverage Δ
CI-GHA 98.69% <100.00%> (+<0.01%) ⬆️
MyPy 83.59% <44.44%> (-0.11%) ⬇️
pytest 100.00% <100.00%> (ø)

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.

@bdraco bdraco deleted the fix_segfault_PyType_GetModuleByDef branch April 9, 2025 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asvetlov asvetlov asvetlov approved these changes

@webknjaz webknjaz Awaiting requested review from webknjaz webknjaz is a code owner

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Segmentation fault on init with Python 3.10 (likely 3.9 as well) when MultiDict is subclassed (3.11,3.12,3.13 not affected)

2 participants

@bdraco @asvetlov