FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-46417: Finalize structseq types at exit by vstinner · Pull Request #30645 · python/cpython · GitHub

/ cpython Public

bpo-46417: Finalize structseq types at exit - #30645

Merged
vstinner merged 5 commits into
python:mainfrom
vstinner:WIP_type_fini
Jan 21, 2022
ConversationCommits (5)ChecksFiles changed
Merged

bpo-46417: Finalize structseq types at exit#30645
vstinner merged 5 commits into
python:mainfrom
vstinner:WIP_type_fini

Conversation

vstinner commented Jan 17, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Add _PyStructSequence_FiniType() and _PyStaticType_Dealloc()
functions to finalize a structseq static type in Py_Finalize().
Currrently, these functions do nothing if Python is built in release
mode.

Clear static types:

  • AsyncGenHooksType: sys.set_asyncgen_hooks()
  • FlagsType: sys.flags
  • FloatInfoType: sys.float_info
  • Hash_InfoType: sys.hash_info
  • Int_InfoType: sys.int_info
  • ThreadInfoType: sys.thread_info
  • UnraisableHookArgsType: sys.unraisablehook
  • VersionInfoType: sys.version
  • WindowsVersionType: sys.getwindowsversion()

https://bugs.python.org/issue46417

vstinner commented Jan 17, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

There are a few more structseq types:

  • Modules/_cursesmodule.c: NcursesVersionType
  • Modules/_threadmodule.c: ExceptHookArgsType
  • Modules/signalmodule.c: SiginfoType
  • Modules/timemodule.c: StructTimeType
  • Objects/longobject.c: Int_InfoType <= DONE
  • Python/errors.c: UnraisableHookArgsType <= DONE
  • Python/sysmodule.c: FlagsType <= DONE
  • Python/sysmodule.c: VersionInfoType <= DONE
  • Python/sysmodule.c: WindowsVersionType <= DONE
  • Python/thread.c: ThreadInfoType <= DONE

vstinner commented Jan 17, 2022

Copy link
Copy Markdown
Member Author

cc @erlend-aasland @corona10

shihai1991 reviewed Jan 19, 2022
View reviewed changes
Comment thread Objects/typeobject.c Outdated

vstinner commented Jan 19, 2022

Copy link
Copy Markdown
Member Author

I changed my mind and also enabled the code on Python release build. Previously, I only enabled the code if the Py_DEBUG macro was defined. For example, the Windows CI job on pull requests builds Python in release mode. If something is wrong, I prefer to catch the bug early and fix it.

bpo-46417: Finalize structseq types at exit
9987879 
Add _PyStructSequence_FiniType() and _PyStaticType_Dealloc()
functions to finalize a structseq static type in Py_Finalize().
Currrently, these functions do nothing if Python is built in release
mode.

Clear static types:

* AsyncGenHooksType: sys.set_asyncgen_hooks()
* FlagsType: sys.flags
* FloatInfoType: sys.float_info
* Hash_InfoType: sys.hash_info
* Int_InfoType: sys.int_info
* ThreadInfoType: sys.thread_info
* UnraisableHookArgsType: sys.unraisablehook
* VersionInfoType: sys.version
* WindowsVersionType: sys.getwindowsversion()
vstinner changed the title bpo-46417: Finalize a few structseq types at exit bpo-46417: Finalize structseq types at exit Jan 19, 2022
vstinner marked this pull request as ready for review January 19, 2022 16:09
vstinner requested a review from markshannon as a code owner January 19, 2022 16:09

vstinner commented Jan 19, 2022

Copy link
Copy Markdown
Member Author

The PR is now ready for a review :-)

shihai1991 approved these changes Jan 20, 2022
View reviewed changes

shihai1991 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

OK. This PR looks good to me :)
But I think another question: Can we add a global cache to maintain the StructSequenceType and don't call the dealloc function in finalize_interp_types() manually?

vstinner commented Jan 20, 2022

Copy link
Copy Markdown
Member Author

Can we add a global cache to maintain the StructSequenceType and don't call the dealloc function in finalize_interp_types() manually?

Do you mean to create a list of structseq types when _PyStructSequence_InitType() or _PyStructSequence_InitType2() is called? Yeah, I had this idea, but I am not sure about it.

A problem is to control exactly when these types are deleted and types them in a specific order. The Python finalization is quite complex, see my notes about past issues: https://pythondev.readthedocs.io/finalization.html

assert(Py_REFCNT(type) == 1); of _PyStructSequence_FiniType() fails if something still keeps the a reference to the type somewhere: if the type is deallocated "too early".

shihai1991 commented Jan 20, 2022

Copy link
Copy Markdown
Member

Can we add a global cache to maintain the StructSequenceType and don't call the dealloc function in finalize_interp_types() manually?

Do you mean to create a list of structseq types when _PyStructSequence_InitType() or _PyStructSequence_InitType2() is called? Yeah, I had this idea, but I am not sure about it.

Yes. That's what I mean : )

A problem is to control exactly when these types are deleted and types them in a specific order. The Python finalization is quite complex, see my notes about past issues: https://pythondev.readthedocs.io/finalization.html

assert(Py_REFCNT(type) == 1); of _PyStructSequence_FiniType() fails if something still keeps the a reference to the type somewhere: if the type is deallocated "too early".

OK, Thanks for your information. I will study your notes.

corona10 approved these changes Jan 20, 2022
View reviewed changes

corona10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I am +1 on this change :)

but same as @encukou, we may need tests to sure if the types are correctly reinitialized if there are multiple finalize/init cycles

vstinner commented Jan 20, 2022

Copy link
Copy Markdown
Member Author

@corona10:

but same as @encukou, we may need tests to sure if the types are correctly reinitialized if there are multiple finalize/init cycles

I added tests. Would you mind to review these tests as well?

vstinner commented Jan 20, 2022

Copy link
Copy Markdown
Member Author

I enhanced the tests to check more PyTypeObject members.

vstinner merged commit e9e3eab into python:main Jan 21, 2022
vstinner deleted the WIP_type_fini branch January 21, 2022 00:42
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

corona10 corona10 approved these changes
shihai1991 shihai1991 approved these changes
markshannon Awaiting requested review from markshannon markshannon is a code owner

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL