🌐 AI搜索 & 代理 主页
Skip to content

Conversation

@filmor
Copy link
Member

@filmor filmor commented Aug 9, 2025

Main changes:

TODO:

  • Reenable all tests
  • Ensure that there are no crashes during garbage collection
  • Ensure that the meta type uses the correct (i.e.: our) tp_setattro instead of the generic one

@kunom
Copy link

kunom commented Oct 16, 2025

Some subjective feedback from my Windows machine with VS 2022:

@guywithface
Copy link

guywithface commented Oct 16, 2025

Also had to replace the target framework from net6.0 to net8.0. Also some feedback for .net 8:

  • Add #pragma warning disable SYSLIB0011 for where BinaryFormatter is being used
  • Add <EnableUnsafeBinaryFormatterSerialization>true</EnableUnsafeBinaryFormatterSerialization> to all projects
  • Remove NonCopyable from ref struct NewReference
  • Remove NonCopyable from ref struct StolenReference
  • Need to consider migrating away from using BinaryFormatter as it will be deprecated in .net9

And now getting an access memory violation for PyGC_Collect(); in Runtime.cs

@filmor
Copy link
Member Author

filmor commented Oct 23, 2025

Status report: After applying the alignment "fix" (thanks!), things work for the most part. There are some crashes in explicit garbage collection, so I have disabled those tests for now.

@filmor
Copy link
Member Author

filmor commented Oct 24, 2025

Also, we currently apparently have a weird inheritance order for our meta type, which leads to it using the PyObject_GenericSetAttr instead of type_setattro, which in turn triggers assertions since python/cpython@5a1618a

@kunom
Copy link

kunom commented Oct 31, 2025

@filmor As the CI seems to be pretty broken (pytest: command not found), could you please provide a list of the outstanding tasks for this PR to reach completion, as you did in the description block of the corresponding 3.13 PR.

Things not clear to me:

  • do you want to have the exluded tests re-enabled?
  • do the tests failing due to missing BinarySerializer functionality block this PR?

Thanks ;-)

@filmor
Copy link
Member Author

filmor commented Oct 31, 2025

There you go. There are still pending issues with the embedding tests that are being worked on in #2638, but I'd be fine with releasing a wheel if we get to the point that all Python tests run through without crashes on 3.14.

@andre-romano

This comment has been minimized.

@filmor
Copy link
Member Author

filmor commented Dec 7, 2025

@filmor
Copy link
Member Author

filmor commented Dec 7, 2025

@lostmsu Please have a look. It will require quite a bit more work to get everything green again (CI-wise, in particular around the embedding), but with these changes, we should be able to release a wheel for Python 3.14.

@filmor filmor requested a review from lostmsu December 7, 2025 14:09
@filmor filmor force-pushed the python3.14 branch 2 times, most recently from 027eb12 to 7ca54ff Compare December 8, 2025 10:24
@filmor filmor marked this pull request as ready for review December 8, 2025 10:29
Copy link
Member

@lostmsu lostmsu left a comment

Choose a reason for hiding this comment

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

LGTM, minor comments only.

I noticed a few files have been reformatted (whitespace only). Perhaps undo that. Technically all reformatting should be in separate commits mentioned in .git-blame-ignore-revs

- name: Embedding tests (.NET Core)
run: dotnet test --runtime any-${{ matrix.os.platform }} --framework net8.0 --logger "console;verbosity=detailed" src/embed_tests/
if: always()
if: false
Copy link
Member

Choose a reason for hiding this comment

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

Can't we limit the skip to 3.14?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a leftover, will reactivate the tests today.

@filmor filmor force-pushed the python3.14 branch 2 times, most recently from 47378be to 3af5779 Compare December 8, 2025 19:36
Not at all sure why this helps, but when assigning `None` instead, the
object is gone at the time of garbage collection.
- The first two run into issues because our meta type's `tp_getattro` is
  not overridden by its subclasses
- The last runs into a `PyErrOccurred` when trying to access its
  `__len__`, related to our sequence decoding logic
In Python 3.14, the objects __dict__ seems to already be half
deconstructed, leading to crashes during garbage collection.

Since gc in Python is single-threaded (I think :)), it should
be fine to have a single static for this. If that is not true,
we can always use a thread-local instead.
Python 3.14 introduced a new assertion that prevents us from using
PyObject_GenericSetAttr directly in our meta type. To work around
this, we manipulate the type dict directly.

This workaround is a simplified variant of Cython's workaround from
cython/cython#6325.

The relevant Python change is in
python/cpython#118454
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.

6 participants