Skip to content

Conversation

@koubaa
Copy link
Contributor

@koubaakoubaa commented Nov 4, 2020

This helps prepare for multiphase init (PEP 489)

https://bugs.python.org/issue1635741

@koubaa
Copy link
ContributorAuthor

@vstinner please review

Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

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

Please mention the modified module name in the PR title.

I suggest to canghe the PR title to "Enhance _datetime error handling" or "Refactor _datetime error handling", since this change is not directly related to multi-phase init. I'm not sure that we can soon convert this module to the multi-phase init because of the PyCapsule C API.

@vstinner
Copy link
Member

You wrote the wrong bpo number in the PR title.

@pganssle
Copy link
Member

@koubaa Can you provide a TL;DR of why this change is useful or necessary?

@vstinner
Copy link
Member

@koubaa Can you provide a TL;DR of why this change is useful or necessary?

https://bugs.python.org/issue1635741 describes the overall goal: better embed Python in application. One side of the issue is to ensure that an extension module releases all its resources at Python exit.

@asvetlovasvetlov changed the title bpo-1536741: refactoring before multi-phase initbpo-1536741: refactoring datetime before multi-phase initNov 4, 2020
@koubaakoubaa changed the title bpo-1536741: refactoring datetime before multi-phase initbpo-1635741: Enhance _datetime error handlingNov 7, 2020
@koubaakoubaaforce-pushed the bpo-1635741-datetime branch from 8910cc2 to 04889eaCompareNovember 7, 2020 21:37
@koubaakoubaaforce-pushed the bpo-1635741-datetime branch from 04889ea to ae49eafCompareNovember 15, 2020 17:55
Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

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

Be careful with preprocessor traps :-( https://gcc.gnu.org/onlinedocs/cpp/Macro-Pitfalls.html

@koubaa
Copy link
ContributorAuthor

Be careful with preprocessor traps :-( https://gcc.gnu.org/onlinedocs/cpp/Macro-Pitfalls.html

Thanks again for sharing this, I easily forget them.

@vstinner
Copy link
Member

Thanks again for sharing this, I easily forget them.

Oh, me tee! I got exactly the same issue than you a few days ago :-D

@vstinnervstinner merged commit 2db8e35 into python:masterNov 20, 2020
@vstinner
Copy link
Member

Thanks, I merged your PR.

I checked manually for refleak using the following test run with "./python -m test (...) -R 3:3":

 def test_leak(self): support.run_in_subinterp("import _datetime") 

I didn't notice any refleak.

@vstinner
Copy link
Member

Hum, I'm not sure that my manual test is relevant, since _datetime doesn't use multiphase init yet. But it leaks, I don't expect issues with Refleaks buildbots.

adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@koubaa@vstinner@pganssle@the-knights-who-say-ni@bedevere-bot