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 — gh-104090: Fix unittest collectedDurations resources leak by bityob · Pull Request #106795 · python/cpython

Conversation

@bityob

@bityob bityob commented Jul 15, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor
  1. Fixed TestResult.addDuration method to add only test repr string and not the test object itself, to avoid resources leak

@iritkatriel

  • Issue: Leaked semaphore objects in test_concurrent_futures #104090

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gpshead reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@bedevere-bot bedevere-bot added the awaiting review label Jul 15, 2023
@bedevere-bot

bedevere-bot commented Jul 15, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot mentioned this pull request Jul 15, 2023
Leaked semaphore objects in test_concurrent_futures #104090
Open
@bityob
bityob force-pushed the gh-104090-fix-leaked-semaphors-on-test_concurrent_futures branch from cdbe1c4 to 8cd162f Compare July 16, 2023 08:53
@iritkatriel

iritkatriel commented Jul 16, 2023

Copy link
Copy Markdown
Member

I think it might be better to split this into two PRs (attached to the same issue), one for the unittest fix and the other for the resource tracker change.
bityob 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.
@bityob
pythongh-104090: Fix unittest collectedDurations resources leak
509636a
@bityob
bityob force-pushed the gh-104090-fix-leaked-semaphors-on-test_concurrent_futures branch from 5585599 to 509636a Compare July 16, 2023 11:58
@bedevere-bot

bedevere-bot commented Jul 16, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bityob
Added NEWS file
ea4e766
@bityob

bityob commented Jul 16, 2023

Copy link
Copy Markdown
Contributor Author

I think it might be better to split this into two PRs (attached to the same issue), one for the unittest fix and the other for the resource tracker change.

Done

Second PR: #106807
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iritkatriel iritkatriel added tests Tests in the Lib/test dir stdlib Standard Library Python modules in the Lib/ directory labels Jul 16, 2023
@iritkatriel

iritkatriel commented Jul 16, 2023

Copy link
Copy Markdown
Member

CC unittest maintainers. @gpshead @ezio-melotti

See issue for motivation.
All reactions

Sorry, something went wrong.

Uh oh!

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

iritkatriel commented Jul 17, 2023

Copy link
Copy Markdown
Member

@giampaolo addDuration was added in 3.12. Should this be backported as a bugfix?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iritkatriel
iritkatriel requested a review from giampaolo July 17, 2023 15:10
@bityob
Merge branch 'main' into pythongh-104090-fix-leaked-semaphors-on-test…
d493239 
…_concurrent_futures
@sunmy2019

sunmy2019 commented Jul 18, 2023

Copy link
Copy Markdown
Member

Is this new entry necessary?

Should this be backported as a bugfix?

+1 from me
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sunmy2019 sunmy2019 added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 18, 2023
@bedevere-bot

bedevere-bot commented Jul 18, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @sunmy2019 for commit d493239 🤖

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 18, 2023
@iritkatriel iritkatriel added the needs backport to 3.12 only security fixes label Jul 18, 2023
@sunmy2019 sunmy2019 mentioned this pull request Jul 19, 2023
gh-104090: Update Resource Tracker to return exit code based on resource leaked found or not #106807
Closed
iritkatriel
iritkatriel reviewed Jul 19, 2023
View reviewed changes
Comment thread Misc/NEWS.d/next/Tests/2023-07-16-02-57-08.gh-issue-104090.cKtK7g.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@iritkatriel
tweak wording in news
800434e
iritkatriel
iritkatriel approved these changes Jul 19, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Jul 19, 2023
@giampaolo

giampaolo commented Jul 19, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

@iritkatriel wrote:

@giampaolo addDuration was added in 3.12. Should this be backported as a bugfix?

Hi! Yes, I think it should be backported.
iritkatriel 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.
@iritkatriel
iritkatriel merged commit 70b961e into python:main Jul 19, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Jul 19, 2023
@miss-islington

miss-islington commented Jul 19, 2023

Copy link
Copy Markdown
Contributor

Thanks @bityob for the PR, and @iritkatriel for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented Jul 19, 2023

Copy link
Copy Markdown

GH-106888 is a backport of this pull request to the 3.12 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the needs backport to 3.12 only security fixes label Jul 19, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 19, 2023
@bityob @miss-islington
pythongh-104090: Fix unittest collectedDurations resources leak (pyth…
8b1fdec 
…onGH-106795)

(cherry picked from commit 70b961e)

Co-authored-by: Yonatan Bitton <[email protected]>
@iritkatriel iritkatriel added the needs backport to 3.12 only security fixes label Jul 19, 2023
@miss-islington

miss-islington commented Jul 19, 2023

Copy link
Copy Markdown
Contributor

Thanks @bityob for the PR, and @iritkatriel for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 19, 2023
@bityob @miss-islington
pythongh-104090: Fix unittest collectedDurations resources leak (pyth…
e0a4576 
…onGH-106795)

(cherry picked from commit 70b961e)

Co-authored-by: Yonatan Bitton <[email protected]>
iritkatriel pushed a commit that referenced this pull request Jul 19, 2023
@miss-islington @bityob
[3.12] gh-104090: Fix unittest collectedDurations resources leak (GH-…
b1c50b8 
…106795) (#106888)

gh-104090: Fix unittest collectedDurations resources leak (GH-106795)
(cherry picked from commit 70b961e)

Co-authored-by: Yonatan Bitton <[email protected]>
@gpshead

gpshead commented Jul 19, 2023

Copy link
Copy Markdown
Member

thanks for figuring this out!
bityob 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.
@picnixz picnixz removed the needs backport to 3.12 only security fixes label Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iritkatriel iritkatriel iritkatriel approved these changes
@giampaolo giampaolo Awaiting requested review from giampaolo

Uh oh!

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

Assignees

No one assigned

Labels

stdlib Standard Library Python modules in the Lib/ directory tests Tests in the Lib/test dir

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.

8 participants

@bityob @bedevere-bot @iritkatriel @sunmy2019 @giampaolo @miss-islington @gpshead @picnixz
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 01402520 — 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.