Skip to content
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

event: Don't use _PyGen_FetchStopIterationValue in Python 3.13+ #1526

Closed
wants to merge 1 commit into from

Conversation

darktohka
Copy link
Contributor

Issue description

Python 3.13 has unpublished the _PyGen_FetchStopIterationValue private C API call. It is now part of the internal API: python/cpython#106320

Solution description

This pull request replaces the private _PyGen_FetchStopIterationValue API call with the new PyErr_GetRaisedException API call, added in Python 3.12.

Because the versioning is so complicated (PyErr_GetRaisedException being added in Python 3.12, _PyGen_FetchStopIterationValue not working in Python 3.13, older versions need to be supported still) I have decided to do the following:

  • Keep the old behaviour on versions older than Python 3.13, as it works perfectly fine
  • Use PyErr_GetRaisedException on Python 3.13+

Right now this code is not covered by the test suite, given that Python 3.13 is still in alpha.

Checklist

I have done my best to ensure that…

  • …I have familiarized myself with the CONTRIBUTING.md file
  • …this change follows the coding style and design patterns of the codebase
  • …I own the intellectual property rights to this code
  • …the intent of this change is clearly explained
  • …existing uses of the Panda3D API are not broken
  • …the changed code is adequately covered by the test suite, where possible.

Copy link
Member

@rdb rdb left a comment

Choose a reason for hiding this comment

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

Thanks.

Should we check for whether the stored value may be nullptr? Right now that would result in a crash.

I agree with only enabling this in Python 3.13. Have you run the test suite with the new code enabled on an older version just to make sure it works, though?

@rdb
Copy link
Member

rdb commented Aug 5, 2023

Looking at this in more detail, I don't think this fix is right. result also needs to be populated if _function != nullptr, used further down in the function, otherwise there won't be a valid DoneStatus extracted for coroutine tasks:

from direct.directbase import DirectStart
from direct.task.Task import Task
import sys

cont = Task.cont

async def thingy(task):
    print(sys.getrefcount(cont))
    return cont

taskMgr.add(thingy)

base.run()

I'll check something in for 1.10.

@rdb rdb self-assigned this Aug 5, 2023
@darktohka
Copy link
Contributor Author

Thank you! I'll let you take things from here then.

@rdb rdb closed this in 0bc290e Sep 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants