fix: python.None.valueOf should return null #88
Conversation
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
This commit resolves memory management issues with Python callbacks, including segfaults, premature callback destruction, and resource leaks. Callbacks had multiple lifetime management issues: 1. Callbacks were freed prematurely when JS PyObject wrappers were GC'd, even though Python still held references (causing segfaults) 2. The same Python object could have multiple PyObject wrappers, each registering with refregistry and causing duplicate Py_DecRef calls 3. Resource leaks occurred when callbacks were never properly cleaned up 4. Auto-created callbacks (from JS functions) had no cleanup mechanism Implement PyCapsule-based callback cleanup: 1. **PyCapsule destructor**: When creating a callback, we: - Create a PyCapsule containing the callback's handle - Use the capsule as the m_self parameter of PyCFunction_NewEx - Register a destructor that Python calls when freeing the PyCFunction - In the destructor, remove the callback from callbackRegistry and destroy it 2. **Separate lifetime tracking**: Callbacks use callbackRegistry instead of refregistry. They are NOT registered with refregistry because: - The initial reference from PyCFunction_NewEx is sufficient - Cleanup happens via the capsule destructor, not FinalizationRegistry - This prevents artificial refcount inflation 3. **Handle-based deduplication**: Track registered handles in a Set (not PyObject instances) to prevent duplicate registration when multiple PyObject wrappers exist for the same Python object 4. **Auto-created callback cleanup**: callbackCleanupRegistry handles callbacks created from raw JS functions that are never passed to Python - src/python.ts: Implement PyCapsule-based callback cleanup - src/symbols.ts: Add PyCapsule_New and PyCapsule_GetPointer symbols - test/test_with_gc.ts: Add proper cleanup to tests All existing tests pass with resource sanitization on
…tration Callbacks are now only added to callbackRegistry when .owned is called (i.e., when actually passed to Python), not during creation. This allows callbackCleanupRegistry to properly clean up callbacks that are created but never passed to Python. Changes: - Store Callback reference in PyObject.#callback during creation - Register with callbackRegistry in .owned getter when callback is passed to Python - Add test case for auto-created callbacks that are never passed to Python
|
the relevant commit is 7b4d6f0 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
built on top of #87