Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-75459: Doc: C API: Improve object life cycle documentation#125962

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
encukou merged 29 commits intopython:mainfromrhansen:docs
May 20, 2025

Conversation

rhansen
Copy link
Contributor

@rhansenrhansen commentedOct 25, 2024
edited by github-actionsbot
Loading

  • Add "cyclic isolate" to the glossary.
  • Add a new "Object Life Cycle" page.
    • Illustrate the order of life cycle functions.
    • DocumentPyObject_CallFinalizer andPyObject_CallFinalizerFromDealloc.
  • PyObject_Init does not calltp_init.
  • PyObject_New:
    • also initializes the memory
    • does not calltp_alloc,tp_new, ortp_init
    • should not be used for GC-enabled objects
    • memory must be freed byPyObject_Free
  • PyObject_GC_New memory must be freed byPyObject_GC_Del.
  • Warn that garbage collector functions can be called from any thread.
  • tp_finalize andtp_clear:
    • Only called when there's a cyclic isolate.
    • Only one object in the cyclic isolate is finalized/cleared at a time.
    • Clearly warn that they might not be called.
    • They can optionally be manually called fromtp_dealloc (viaPyObject_CallFinalizerFromDealloc in the case oftp_finalize).
  • tp_finalize:
    • Referenceobject.__del__.
    • The finalizer can resurrect the object.
    • SuggestPyErr_GetRaisedException andPyErr_SetRaisedException instead of the deprecatedPyErr_Fetch andPyErr_Restore functions.
    • Add links toPyErr_GetRaisedException andPyErr_SetRaisedException.
    • Suggest usingPyErr_WriteUnraisable if an exception is raised during finalization.
    • Rename the example function fromlocal_finalize tofoo_finalize for consistency with thetp_dealloc documentation and as a hint that the name isn't special.
    • Minor wording and sylistic tweaks.
    • Warn thattp_finalize can be called during shutdown.

📚 Documentation preview 📚:https://cpython-previews--125962.org.readthedocs.build/

@rhansen
Copy link
ContributorAuthor

I'm not familiar enough with CPython's internals to be super confident about these changes. I would appreciate it if a GC expert would carefully review this.

Thanks!

@hugovk
Copy link
Member

Hmm, I'm not sure if we can require graphviz for the docs.

We'd have to consider installing it on the main docs server in addition to Read the Docs, and also make sure the docs can still build without it, for downstream redistributors who might only want to build with "vanilla" Sphinx and no extra extensions. Plus other developers would need an easy way to build the docs on their machines.

cc@AA-Turner

@rhansenrhansenforce-pushed thedocs branch 4 times, most recently frombe22691 tod804c6cCompareOctober 25, 2024 09:10
@rhansen
Copy link
ContributorAuthor

Hmm, I'm not sure if we can require graphviz for the docs.

Maybe I should just commit the generated.svg (and the input dot file so it can be revised easily). Would that be acceptable?

encukou reacted with thumbs up emoji

  * Add "cyclic isolate" to the glossary.  * Add a new "Object Life Cycle" page.    * Illustrate the order of life cycle functions.    * Document `PyObject_CallFinalizer` and      `PyObject_CallFinalizerFromDealloc`.  * `PyObject_Init` does not call `tp_init`.  * `PyObject_New`:    * also initializes the memory    * does not call `tp_alloc`, `tp_new`, or `tp_init`    * should not be used for GC-enabled objects    * memory must be freed by `PyObject_Free`  * `PyObject_GC_New` memory must be freed by `PyObject_GC_Del`.  * Warn that garbage collector functions can be called from any    thread.  * `tp_finalize` and `tp_clear`:    * Only called when there's a cyclic isolate.    * Only one object in the cyclic isolate is finalized/cleared at a      time.    * Clearly warn that they might not be called.    * They can optionally be manually called from `tp_dealloc` (via      `PyObject_CallFinalizerFromDealloc` in the case of      `tp_finalize`).  * `tp_finalize`:    * Reference `object.__del__`.    * The finalizer can resurrect the object.    * Suggest `PyErr_GetRaisedException` and      `PyErr_SetRaisedException` instead of the deprecated      `PyErr_Fetch` and `PyErr_Restore` functions.    * Add links to `PyErr_GetRaisedException` and      `PyErr_SetRaisedException`.    * Suggest using `PyErr_WriteUnraisable` if an exception is raised      during finalization.    * Rename the example function from `local_finalize` to      `foo_finalize` for consistency with the `tp_dealloc`      documentation and as a hint that the name isn't special.    * Minor wording and sylistic tweaks.    * Warn that `tp_finalize` can be called during shutdown.
@rhansen
Copy link
ContributorAuthor

I committed the generated.svg so that the substance of this PR can be reviewed while we figure out if it is acceptable to add graphviz as a dependency. (Note thatsphinx.ext.graphviz is a built-in extension, so enabling it doesn't add any new sphinx dependencies.)

@AA-Turner
Copy link
Member

I think that requiringgraphviz should be fine -- Debian, Fedora, Gentoo, and OpenSUSE all package it. As Richard notes, it's a built-in extension, so should be fine from the "Vanilla" perspective.

I would want to include a NEWS entry to say thatgraphviz is now required to build the docs, though.

A

@AA-TurnerAA-Turner added the docsDocumentation in the Doc dir labelOct 25, 2024
@ZeroIntensity
Copy link
Member

My main concern with documenting nitty-gritty details of the lifecycle is that we're technically documenting implementation details, which are subject to change (and we've been bad at updating these kind of things from version-to-version in past). I suggest the SVG go into theInternalDocs folder instead.

It's also worth noting here thattp_finalize isn't 100% related to garbage collection, it's supposed to be used overtp_dealloc if complicated things are being done upon finalization, even for non-GC types. And while we're here, I think it would be a good idea to document the cases thattp_clear should exist for a tracked type.

@rhansen
Copy link
ContributorAuthor

My main concern with documenting nitty-gritty details of the lifecycle is that we're technically documenting implementation details, which are subject to change (and we've been bad at updating these kind of things from version-to-version in past). I suggest the SVG go into theInternalDocs folder instead.

I don't want to document any implementation details here, so I'm happy to remove what isn't necessary. It's hard to tell what is and isn't necessary because the end of an object's life is especially fraught with peril. I think that it is better to err on the side of over-documenting this topic than under-documenting.

I wrote this PR because there were several things that I needed to know that the existing documentation didn't make clear:

  • The order thetp_* functions might be called (to know what invariants are possible).
  • Which threads might execute the functions (for locking correctness).
  • Details about when a function is called (or not) that are necessary for locking correctness. (e.g., if multiple objects in the same cyclic isolate are never finalized concurrently then a lock-free design might be possible)
  • Approximately how often atp_* function might be called: maybe never, exactly once, at most once, at least once, very frequently, etc.
  • Which other objects might be in an inconsistent state.

It's also worth noting here thattp_finalize isn't 100% related to garbage collection, it's supposed to be used overtp_dealloc if complicated things are being done upon finalization, even for non-GC types.

If I understand correctly,tp_finalize is never called for non-GC types unless the class designer calls it fromtp_dealloc. In that casetp_finalize is just like any other helper function that might be called fromtp_dealloc. (Maybe this is only true forstatic types and notheap types? I don't fully understand the difference.)

And while we're here, I think it would be a good idea to document the cases thattp_clear should exist for a tracked type.

I thought that was already sufficiently explained, even before this PR. Can you explain what you think is lacking?

@ZeroIntensity
Copy link
Member

First, thanks for doing this!

I think that it is better to err on the side of over-documenting this topic than under-documenting.

I don't, that limits our ability to modify the lifecycle in the future (especially because there's no good way to deprecate things here). I'll point this out when doing a more in-depth review though, I don't see anything particularly bad right now.

If I understand correctly,tp_finalize is never called for non-GC types unless the class designer calls it fromtp_dealloc.

You're right, it's not, but I don't think we should limit ourselves to that in the future. It might be possible someday to automatically do this for untracked types as well. We should just document thatall types, even GC, requirePyObject_CallFinalizerFromDealloc in the destructor if they wanttp_finalize to geteventually called--we can note that it could happen automatically, though.

I thought that was already sufficiently explained, even before this PR. Can you explain what you think is lacking?

Basically, it's not documented which types need to have atp_clear, because not all GC types have it.I'm not even sure which cases require it. I think it's only needed if the type can have a direct reference cycle to itself? (As in, running its finalizer will try toPy_DECREF itself.)

Also, I don't think it should be documented thattp_clear is related totp_dealloc by making an "optional call", that's sort of incidental. They tend to do the same thing, and the destructor can utilize the clear function for convenience, but they're for different purposes.


A few other notes:

  • It's fine to document what the specific allocators (e.g.PyObject_GC_New andPyObject_GC_Del) do, but we should point users to usingtp_alloc andtp_free instead.
  • That said, I don't see the need to mention that functions likePyObject_New don't calltp_init. Those are strictly allocators, and currently documented as such.
  • This isn't exhaustive--static objects and some immortal objects don't follow this lifecycle (single-phase modules don't either, I think). Other objects might not follow this in the future either.
  • If I read "called from any thread" in the docs, I would be worried about holding the GIL. Maybe mention that while it can be called from any thread, it will still hold the GIL.

@rhansen
Copy link
ContributorAuthor

I did a major rewrite to hopefully address all of the feedback (thanks for reviewing!). Please take another look.

@ZeroIntensity
Copy link
Member

I'm going to be taking this over. I've addressed my own comments and most of Petr's. Other reviews would be appreciated!

@encukou
Copy link
Member

Thank you! I definitely want to take another look -- after Beta 1, next week or so.

Copy link
Member

@encukouencukou left a comment

Choose a reason for hiding this comment

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

Overall, this is an improvement. Thank you for the work!
Let's do one more pass, merge, and leave the rest for a future PR?

Co-authored-by: Petr Viktorin <encukou@gmail.com>
@ZeroIntensity
Copy link
Member

Let's do one more pass, merge, and leave the rest for a future PR?

Sounds good to me.

@encukouencukou merged commit3246ea5 intopython:mainMay 20, 2025
39 checks passed
@github-project-automationgithub-project-automationbot moved this fromTodo toDone inDocs PRsMay 20, 2025
@ZeroIntensity
Copy link
Member

Thanks, Petr!

Do we want to backport this? On the one hand, this is a great improvement to the C API documentation that can definitely help users in 3.13. But on the other day, this is a huge change to backport. Maybe that doesn't matter? I'm not sure we should treat documentation changes the same way we treat code changes.

@hugovk
Copy link
Member

Very few people read the pre-release docs. Over the past week, we had these many visitors for these homepages:

  • /3/ - 94.1k
  • /3.13/ - 33.6k (same as/3/)
  • /3.14/ - 2.3k
  • /3.15/ - 4.4k
  • /dev/ - 358 (same as/3.15)

If we want people to benefit from this work before October 2026(!), we should backport.

@ZeroIntensity
Copy link
Member

To 3.14, or all the way to 3.13? I can see arguments for both. Note that if we backport to 3.13, we also need to backport theattached thread state change that this PR uses. (That might be a good thing, I've noticed myself constantly pointing free-threading users to the 3.14 docs for the better documentation.)

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotiOS ARM64 Simulator 3.x (tier-3) has failed when building commit3246ea5.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1380/builds/3754) and take a look at the build logs.
  4. Check if the failure is related to this commit (3246ea5) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1380/builds/3754

Failed tests:

  • test.test_concurrent_futures.test_interpreter_pool

Failed subtests:

  • test_map_buffersize_on_infinite_iterable - test.test_concurrent_futures.test_interpreter_pool.InterpreterPoolExecutorTest.test_map_buffersize_on_infinite_iterable

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):  File"/Users/buildbot/Library/Developer/XCTestDevices/4832ED11-0C79-4767-B1A6-77FDBE8E03AE/data/Containers/Bundle/Application/95646D17-A682-4984-91B0-88B196595790/iOSTestbed.app/python/lib/python3.15/concurrent/futures/thread.py", line99, in_worker    ctx.initialize()~~~~~~~~~~~~~~^^  File"/Users/buildbot/Library/Developer/XCTestDevices/4832ED11-0C79-4767-B1A6-77FDBE8E03AE/data/Containers/Bundle/Application/95646D17-A682-4984-91B0-88B196595790/iOSTestbed.app/python/lib/python3.15/concurrent/futures/interpreter.py", line132, ininitializeself.interpid= _interpreters.create(reqrefs=True)~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^interpreters.InterpreterError:interpreter creation failedERRORTraceback (most recent call last):  File"/Users/buildbot/Library/Developer/XCTestDevices/4832ED11-0C79-4767-B1A6-77FDBE8E03AE/data/Containers/Bundle/Application/95646D17-A682-4984-91B0-88B196595790/iOSTestbed.app/python/lib/python3.15/test/test_concurrent_futures/executor.py", line127, intest_map_buffersize_on_infinite_iterableself.assertEqual(next(res,None),"0")~~~~^^^^^^^^^^^  File"/Users/buildbot/Library/Developer/XCTestDevices/4832ED11-0C79-4767-B1A6-77FDBE8E03AE/data/Containers/Bundle/Application/95646D17-A682-4984-91B0-88B196595790/iOSTestbed.app/python/lib/python3.15/concurrent/futures/_base.py", line666, inresult_iteratoryield _result_or_cancel(fs.pop())~~~~~~~~~~~~~~~~~^^^^^^^^^^  File"/Users/buildbot/Library/Developer/XCTestDevices/4832ED11-0C79-4767-B1A6-77FDBE8E03AE/data/Containers/Bundle/Application/95646D17-A682-4984-91B0-88B196595790/iOSTestbed.app/python/lib/python3.15/concurrent/futures/_base.py", line311, in_result_or_cancelreturn fut.result(timeout)~~~~~~~~~~^^^^^^^^^  File"/Users/buildbot/Library/Developer/XCTestDevices/4832ED11-0C79-4767-B1A6-77FDBE8E03AE/data/Containers/Bundle/Application/95646D17-A682-4984-91B0-88B196595790/iOSTestbed.app/python/lib/python3.15/concurrent/futures/_base.py", line450, inresultreturnself.__get_result()~~~~~~~~~~~~~~~~~^^  File"/Users/buildbot/Library/Developer/XCTestDevices/4832ED11-0C79-4767-B1A6-77FDBE8E03AE/data/Containers/Bundle/Application/95646D17-A682-4984-91B0-88B196595790/iOSTestbed.app/python/lib/python3.15/concurrent/futures/_base.py", line395, in__get_resultraiseself._exceptionconcurrent.futures.interpreter.BrokenInterpreterPool:A thread initializer failed, the thread pool is not usable anymore

@encukou
Copy link
Member

I'd go for 3.14; that'll become/3/ inthis October.

hugovk and ZeroIntensity reacted with thumbs up emoji

@ZeroIntensityZeroIntensity added the needs backport to 3.14bugs and security fixes labelMay 20, 2025
@miss-islington-app
Copy link

Thanks@rhansen for the PR, and@encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestMay 20, 2025
…ythonGH-125962)  * Add "cyclic isolate" to the glossary.  * Add a new "Object Life Cycle" page.  * Improve docs for related API, with special focus on cross-references and warnings(cherry picked from commit3246ea5)Co-authored-by: Richard Hansen <rhansen@rhansen.org>Co-authored-by: Petr Viktorin <encukou@gmail.com>Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
@bedevere-app
Copy link

GH-134344 is a backport of this pull request to the3.14 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.14bugs and security fixes labelMay 20, 2025
encukou added a commit that referenced this pull requestMay 20, 2025
…H-125962) (GH-134344)gh-75459: Doc: C API: Improve object life cycle documentation (GH-125962)  * Add "cyclic isolate" to the glossary.  * Add a new "Object Life Cycle" page.  * Improve docs for related API, with special focus on cross-references and warnings(cherry picked from commit3246ea5)Co-authored-by: Richard Hansen <rhansen@rhansen.org>Co-authored-by: Petr Viktorin <encukou@gmail.com>Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@encukouencukouencukou left review comments

@hugovkhugovkhugovk left review comments

@AA-TurnerAA-TurnerAA-Turner left review comments

@ZeroIntensityZeroIntensityZeroIntensity left review comments

@ezio-melottiezio-melottiAwaiting requested review from ezio-melotti

Assignees
No one assigned
Labels
docsDocumentation in the Doc dirskip news
Projects
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

7 participants
@rhansen@hugovk@AA-Turner@ZeroIntensity@encukou@bedevere-bot@ezio-melotti

[8]ページ先頭

©2009-2025 Movatter.jp