Repository navigation
Conversation
…ing memory Added an AllocType field on CPPMethod, cached from Cpp::IsAllocator, that now decides the ownership of a returned object. The overloads of a group share fMethodInfo, so the answer of the called overload stays in a local instead of going into the shared kIsCreator flag, which nothing ever clears. The analyzer is not called; this is the attribute-checker path only. AllocType was collapsed into kIsCreator and never reached op_dealloc_nofree, so malloc'd, new[]'d and raw operator-new'd memory were all freed with a scalar delete. It now rides on the instance as kIsArrayAlloc, kIsNoConstruct and kIsMalloc; op_dealloc_nofree branches on them to delete[], free, ::operator delete or ::operator delete[], with no destructor for the raw cases. All flags clear keeps the old delete path. bind_object carries those flags to the new proxy along with the ownership, as BindCppObjectNoCast() masks them out, so a cast object frees its memory the way it was allocated. Mirrored that with Cpp::IsDeallocator, cached the same way, and passed the call arguments to HandleReturn: handing a Python-owned instance to an ownership-taking function now gives ownership back to C++ instead of leaving it to be freed twice. IsDeallocator reports one DeallocType per parameter, so every argument of a call is matched to the parameter that the attribute labels. A positional argument finds its slot from its place in the args array, and a keyword argument from its name in the signature. Two shifts turn either number into a slot, because an implicit object parameter and the shape of the call each move it: GetDeallocArgShift() answers for the args array and GetDeallocParamShift() for the declaration. CPPMethod, CPPFunction and CPPClassMethod answer both for their own kind of callee, and the defaults on PyCallable keep every other callable out of this path. The interop layer carries what the above needs. interop::AllocType and interop::DeallocType name the CppInterOp enums, and IsAllocator() and IsDeallocator() join their neighbours in the wrapper, each holding the interpreter mutex. interop::Destruct() takes an object count, which it used to pin to 0; a count other than 0 makes the CppInterOp destructor wrapper run delete[] in place of delete, which is what the array path wants. CPPINTEROP_GIT_TAG moves to the CppInterOp commit that carries those two entry points, and test/Makefile builds the memory_analysis dictionary. Ownership is only released, never nullified: the instance keeps a stale pointer after the callee frees the memory. GetAllocBehaviour is restricted to raw-pointer returns. Exposed read-only __is_array_alloc__ / __is_no_construct__ / __is_malloc__ and added tests for the deallocation paths and for ownership release via free, via an ownership_takes function, and the negative case. An attribute reaches the tests three ways: a clang::annotate on the declaration, a redeclaration that carries it, and APINotes behind a module map. The release tests cover the bound, unbound and stored-bound forms of a member method, a static method and a free function, each in a positional and a keyword shape, a keyword shape with the names reversed, an argument that is no object at all, a cppDeallocNone parameter, a Python overload that carries no attribute, and a cast through bind_object.
ll_dealloc freed fBuf, the address of the pointer slot, so every owning view leaked its buffer. It now frees get_buf(), and the new kIsNew flag, exposed as __is_new__, separates new, new[] and malloc. HandleReturn takes ownership of a returned view and writes the allocation kind. The release loop clears the ownership of a view argument that the callee frees.
This branch has not been deployed
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
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 upon #114, just second commit is relevant
Changes:
LowLevelView objects now use the ownership attributes. A view that owns its buffer frees it the way the memory was allocated, and a view whose buffer the callee frees gives up its ownership.
ll_deallocimplementation was leaking memory for every case because it was using fBuf every time instead ofget_buf(). It is fixed now and deallocation branches are extended as a new flag is added for carrying information from AllocTypekIsNewflag, read through__is_new__, separatesnewfromnew[]and
malloc.ll_deallocpicks::operator delete[],::operator deleteor
free.HandleReturntakes ownership of a returned view and writes the allocationkind from
AllocType. A creator without an attribute keeps thenew[]assumption of
ll_new.kIsOwnerwhen thecallee frees the buffer.
@Vipul-Cariappa