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.

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

bpo-41994: Fix refcount issues in Python/import.c #22632

Merged
merged 10 commits into from Jan 12, 2021

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Oct 10, 2020

Python/import.c Outdated Show resolved Hide resolved
Python/import.c Outdated Show resolved Hide resolved
Python/import.c Outdated Show resolved Hide resolved
@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 11, 2020
@bedevere-bot
Copy link

bedevere-bot commented Oct 11, 2020

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 25ba819 🤖

If you want to schedule another build, you need to add the "🔨 test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 11, 2020
@serhiy-storchaka
Copy link
Member Author

serhiy-storchaka commented Oct 22, 2020

@ericsnowcurrently, please take a look. It has relation to the code added by you (support of arbitrary mapping as sys.modules).

@github-actions
Copy link

github-actions bot commented Dec 17, 2020

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Dec 17, 2020
Copy link
Member

@encukou encukou left a comment

I went through the changes and they look OK to me!
To review, I needed to merge current master & resolve a small conflict; I hope you don't mind me pushing that into this PR.

@ericsnowcurrently, do you want to review as well? I might have missed some big-picture issue.

@serhiy-storchaka
Copy link
Member Author

serhiy-storchaka commented Dec 29, 2020

Thank you @encukou. I planned to merge with master, but it needed some time because there is easy to miss some detail in large image when change how reference counted. Good opportunity to do one more review.

@serhiy-storchaka serhiy-storchaka added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 29, 2020
@bedevere-bot
Copy link

bedevere-bot commented Dec 29, 2020

🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit 03e405f 🤖

If you want to schedule another build, you need to add the "🔨 test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 29, 2020
@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Dec 30, 2020
@encukou
Copy link
Member

encukou commented Dec 30, 2020

Unfortunately that last merge was with a leaking master, which is now fixed. Another merge should fix the tests.

@serhiy-storchaka serhiy-storchaka added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 30, 2020
@bedevere-bot
Copy link

bedevere-bot commented Dec 30, 2020

🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit c31124f 🤖

If you want to schedule another build, you need to add the "🔨 test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 30, 2020
@encukou
Copy link
Member

encukou commented Jan 5, 2021

And now the tests failed because of bpo-42794. This PR seems cursed :(

@bedevere-bot
Copy link

bedevere-bot commented Jan 5, 2021

🤖 New build scheduled with the buildbot fleet by @encukou for commit bc4ba0c 🤖

If you want to schedule another build, you need to add the "🔨 test-with-buildbots" label again.

@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 5, 2021
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 5, 2021
@encukou encukou merged commit 4db8988 into python:master Jan 12, 2021
58 of 59 checks passed
@miss-islington
Copy link
Contributor

miss-islington commented Jan 12, 2021

Thanks @serhiy-storchaka for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒🤖

@miss-islington
Copy link
Contributor

miss-islington commented Jan 12, 2021

Sorry, @serhiy-storchaka and @encukou, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 4db8988420e0a122d617df741381b0c385af032c 3.9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs backport to 3.9 type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants