Skip to content

bpo-46417: Finalize structseq types at exit #30645

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

Merged
merged 5 commits into from
Jan 21, 2022
Merged

bpo-46417: Finalize structseq types at exit #30645

merged 5 commits into from
Jan 21, 2022

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jan 17, 2022

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
Copy link
Member Author

vstinner commented Jan 17, 2022

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
Copy link
Member Author

cc @erlend-aasland @corona10

@vstinner
Copy link
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.

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 vstinner changed the title bpo-46417: Finalize a few structseq types at exit bpo-46417: Finalize structseq types at exit Jan 19, 2022
@vstinner vstinner marked this pull request as ready for review January 19, 2022 16:09
@vstinner vstinner requested a review from markshannon as a code owner January 19, 2022 16:09
@vstinner
Copy link
Member Author

The PR is now ready for a review :-)

Copy link
Member

@shihai1991 shihai1991 left a comment

Choose a reason for hiding this comment

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

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
Copy link
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
Copy link
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.

Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

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
Copy link
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
Copy link
Member Author

I enhanced the tests to check more PyTypeObject members.

@vstinner vstinner merged commit e9e3eab into python:main Jan 21, 2022
@vstinner vstinner deleted the WIP_type_fini branch January 21, 2022 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants