Override dead bindings when registering a new binding. #75
Conversation
This file contains 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 is needed to handle the case where the underlying C++ object is deleted, but its Python wrapper is still alive. If a new C++ object is allocated at the same address, it needs to override the old binding to prevent the situation where the old binding is reused. If the old binding is not replaced, a C++ object may be bound to a Python wrapper for different object type. (Or worse, a C++ object is bound to a Python object which no longer exists.)
…istered for a C++ object. This allows you to assert that `shiboken.getBinding(shiboken.getCppPointer(obj)[0]) is obj`, which was not true in the issue I was investigating.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This fixes the behavior observed in #74.
When a C++ object is deallocated, the BindingManager retains a binding that points from the C++ object to its Python wrapper. The binding is normally destroyed when the Python wrapper is freed. If a new C++ object is allocated at the same address (before the Python wrapper is freed), the new binding will not be registered.
This change ensures that the new binding will be registered even if the old Python wrapper has not been freed.
When the old Python wrapper is freed, it uses the Python wrapper's address to destroy the binding (not the C++ object's address), so it will not destroy the new binding.
(BindingManager would previously override old bindings. This was changed in commit 3b74744 without explaining why.)
Additionally, this change adds the method
shiboken.getBinding(cppAddress)to the shiboken module to aid in debugging similar problems with the BindingManager.