GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-43901: Fix refleaks in test_module by pablogsal · Pull Request #25754 · python/cpython

Conversation

@pablogsal

@pablogsal pablogsal commented Apr 30, 2021 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

https://bugs.python.org/issue43901

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@pablogsal
bpo-43901: Fix refleaks in test_module
d1554e0
@pablogsal
pablogsal requested a review from larryhastings April 30, 2021 15:59
@bedevere-bot bedevere-bot added the awaiting core review label Apr 30, 2021
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Apr 30, 2021
@pablogsal pablogsal added the skip news label Apr 30, 2021
@pablogsal

pablogsal commented Apr 30, 2021

Copy link
Copy Markdown
Member Author

I'm still missing another leak:

𓋹 ./python.exe -m test test_module -R :
0:00:00 load avg: 2.89 Run tests sequentially
0:00:00 load avg: 2.89 [1/1] test_module
beginning 9 repetitions
123456789
.........
test_module leaked [1, 1, 1, 1] references, sum=4
test_module failed

== Tests result: FAILURE ==

1 test failed:
    test_module

Total duration: 2.0 sec
Tests result: FAILURE
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal pablogsal mentioned this pull request Apr 30, 2021
bpo-43901: Upgrade test to use cool module reloading tech. #25752
Closed
@pablogsal
Fix another leak
3640843
@pablogsal

pablogsal commented Apr 30, 2021

Copy link
Copy Markdown
Member Author

I'm still missing another leak:

Gotcha! I have it fixed in 3640843

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal

pablogsal commented Apr 30, 2021

Copy link
Copy Markdown
Member Author

Ok, we are clean:

𓋹 ./python.exe -m test test_module -R :
0:00:00 load avg: 3.33 Run tests sequentially
0:00:00 load avg: 3.33 [1/1] test_module
beginning 9 repetitions
123456789
.........

== Tests result: SUCCESS ==

1 test OK.

Total duration: 2.0 sec
Tests result: SUCCESS
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

larryhastings
larryhastings approved these changes Apr 30, 2021
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Apr 30, 2021
@larryhastings

larryhastings commented Apr 30, 2021

Copy link
Copy Markdown
Contributor

I was about to make my PR ;-)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal

pablogsal commented Apr 30, 2021

Copy link
Copy Markdown
Member Author

I was about to make my PR ;-)

Sorry, I was doing this patch and missed your comments on BPO :(

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@larryhastings

larryhastings commented Apr 30, 2021

Copy link
Copy Markdown
Contributor

It's hard to see how that code could ever have passed a refleak check! It seems like would always have leaked that dict. Yet I swear I ran one!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal

pablogsal commented Apr 30, 2021

Copy link
Copy Markdown
Member Author

@larryhastings If you already have the PR made I can close this one and we can merge yours if you prefer :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@larryhastings

larryhastings commented Apr 30, 2021

Copy link
Copy Markdown
Contributor

I don't mind the parallel effort, as long as it gets fixed one way or another. I've caused enough headaches and gnashing of teeth this week--let's try and keep this incident to a minimum!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@larryhastings

larryhastings commented Apr 30, 2021

Copy link
Copy Markdown
Contributor

Let's go with yours, the PR is already up and checks are running.

pablogsal reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal

pablogsal commented Apr 30, 2021

Copy link
Copy Markdown
Member Author

I've caused enough headaches and gnashing of teeth this week--let's try and keep this incident to a minimum!

No problem Larry! This happens quite a lot for everyone, and the important thing is that we have your patch for 3.10 :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@larryhastings

larryhastings commented Apr 30, 2021

Copy link
Copy Markdown
Contributor

No problem Larry! This happens quite a lot for everyone,

What rot! This never happened during the release process for 3.4 and 3.5! Nobody ever broke the build, ever. I'm clearly a tiresome outlier.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal
pablogsal merged commit e374a40 into python:master Apr 30, 2021
@bedevere-bot bedevere-bot removed the awaiting merge label Apr 30, 2021
@pablogsal
pablogsal deleted the bpo-43901 branch April 30, 2021 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@larryhastings larryhastings larryhastings approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@pablogsal @larryhastings @the-knights-who-say-ni @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.