Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Segfault for Python 3.11 #293

Merged
merged 1 commit into from
Oct 21, 2023
Merged

Fix Segfault for Python 3.11 #293

merged 1 commit into from
Oct 21, 2023

Conversation

jackyk02
Copy link
Contributor

Issue
Segfault due to Python 3.11 changes to object layout and memory allocation methods.

Background
Python 3.11 introduced changes to optimize CPython. One of the significant changes, as mentioned in this PR, modifies the object layout to place pointers to dict and values immediately before the GC header. This change potentially affected the memory layout of custom python objects.

Before:
Untitled1
After:
Untitled2
Moreover, this commit removed _PyObject_GC_Calloc and _PyObject_GC_Malloc, affecting the memory allocation for objects that participate in garbage collection.

Solution
The new memory allocation relies on PyObject_New instead of PyObject_GC_New.

@edwardalee edwardalee self-requested a review October 21, 2023 16:47
@edwardalee edwardalee added the enhancement Enhancement of existing feature label Oct 21, 2023
edwardalee
edwardalee approved these changes Oct 21, 2023
View reviewed changes
Copy link
Contributor

@edwardalee edwardalee left a comment

Choose a reason for hiding this comment

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

LGTM!

@edwardalee edwardalee merged commit eab52f4 into lf-lang:main Oct 21, 2023
25 checks passed
@petervdonovan
Copy link
Contributor

@Jakio815 seems to have been blocked because this PR causes segfaults in src/federated/DistributedLoopedAction.lf and src/federated/DistributedLoopedPhysicalAction.lf. It is possible that we should run the Python tests as a requirement for merging changes in reactor-c.

@lhstrh
Copy link
Member

lhstrh commented Nov 6, 2023

I also noticed that main of reactor-c is failing, but probably for a different reason.

@petervdonovan petervdonovan mentioned this pull request Nov 6, 2023
Merged
@lhstrh lhstrh mentioned this pull request Nov 7, 2023
Merged
4 tasks
lhstrh added a commit that referenced this pull request Nov 8, 2023
@lhstrh
Revert "Merge pull request #293 from jackyk02/main"
261b405 
This reverts commit eab52f4, reversing
changes made to 654fa0c.
@lhstrh lhstrh mentioned this pull request Nov 8, 2023
Merged
lhstrh added a commit that referenced this pull request Nov 8, 2023
@lhstrh
Merge pull request #303 from lf-lang/python-ci
d82cf9d 
Run Python tests in CI and revert #293
@jackyk02 jackyk02 mentioned this pull request Nov 9, 2023
Open
@lhstrh lhstrh changed the title Fix Segfault for Python 3.11 Fix Segfault for Python 3.11 Nov 9, 2023
@lhstrh lhstrh added exclude Exclude from change log and removed enhancement Enhancement of existing feature labels Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edwardalee edwardalee edwardalee approved these changes

Assignees
No one assigned
Labels
exclude Exclude from change log
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jackyk02 @petervdonovan @lhstrh @edwardalee