[PATCH] pytalloc and pidl changes for refcounting and incorrect msDS-IntID handling

[Stefan Metzmacher]

Hi Andrew,

>>>> Thanks.  I also have had a failure that indicates our python
>>>> reference
>>>> counting issue may be biting us again.  I'm looking into that.
>>>>
>>>> Andrew Bartlett
>>>
>>> The attached patches appear to address the segfaults by sorting out
>>> our
>>> reference handling - we need to track both the talloc context of
>>> the
>>> Samba object and the reference handle used by python.
>>>
>>> I even have tests to show we are handling the references correctly!
>>
>> Thanks!
>>
>> There's one unused tmp_ctx variable you just create and free...
>>
>>> This all seems fairly sane, until you realise that pytalloc.h is a
>>> PUBLIC header.  I've tried some things to fix that, but none of it
>>> is
>>> easy, or even practical, so I'm posting this to the list to seek
>>> advice.  
>>>
>>> Is it reasonable to make an ABI break on pytalloc.h without
>>> changing
>>> the talloc major version?
>>>
>>
>> I think the goal should be to remove *every* PyObject_HEAD from any
>> header
>> file! That means all internal object state is private and instead of
>> ugly
>> macros like pytalloc_get_mem_ctx(), pytalloc_get_ptr(),
>> PyCredentials_AsCliCredentials()
>> and many others need to be real functions.
> 
> I've done this.
> 
>> For objects derived from "talloc.Object", we should provide a
>> pytalloc_Ready() as wrapper to PyType_Ready(), that fills in the
>> required
>> magic to the PyTypeObject.
> 
> I chose to just expose a size function, but yes, this is essentially
> what we now have.
> 
>> In order to prevent ABI problems we can think of adding new functions
>> and keep (and deprecate) the broken once. Maybe we can provide the
>> abstract
>> class as something like "talloc.BaseObject".
> 
> I've added new functions, and used the talloc.BaseObject name.  I've
> removed the macros, which creates an API (but not ABI) break if callers
> used pytalloc_Object as parts of Samba did and build strictly, because
> the types will mismatch.
> 
> I think this is the best way forward, given the limited audience and
> that it avoids leaving the macros around forever. 
> 
> I've added tests, both for the PIDL changes and in test_pytalloc.c

I like the typechecking on pytalloc_get_mem_ctx() and pytalloc_get_ptr(),
which means this is mostly transparent, which simplifies a lot.

I think we can even avoid having a pytalloc_get_ptr_mem_ctx() function
and just let pytalloc_get_mem_ctx() return the talloc_ptr_ctx for
a BaseObject.

I've also some more comments I'll write within the next few days.

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160222/49f88e4b/signature.sig>