The previous fix is incomplete and will leads to seg fault/mgmt operation failures on certain mgmt operations in service bus -- so we revert the change.
Through investigation, I find that the filter_set on cSource in cython also needs to be cloned first as the c module won't clone it when we're getting the value.
two c AMQP_VALUE are cloned while not being destroyed:
cdef c_amqpvalue.AMQP_VALUE cloned_source
cdef c_amqpvalue.AMQP_VALUE cloned_target
destroying them in the finally could correctly decrease the reference count
however random segmentation fault will be triggered.
after keeping investigation into how the properties on the source/targer are used, the address property is returned by source_get_address which is a in_memory opeartion (value not cloned, reference count doesn't + 1), that seems to be place where seg fault took place
it cannot be simply wrapped by the cython AMQPValue (via value_factory), as AMQPValue will call destroy (reference count - 1) when GC kicks in. the reference count will be incorrect at that time.
it should be cloned first, we have other similar codes in amqpvalue.pyx when C *_in_memory methods are called
after fixing this, no seg fault happens
I further searched where value_factory methods are used and found two more places that need fix
the pop method in cdef class CompositeValue(AMQPValue)
the get_body_sequence method in cdef class cMessage
This is the following up PR on the previous memory leak fix: https://github.com/Azure/azure-uamqp-python/pull/215.
Latest updates
filter_setoncSourcein cython also needs to be cloned first as the c module won't clone it when we're getting the value.Previous context
Problem:
Root cause:
Client creation would leak memory after connection and session is established because
on_link_attachedcallback insession.pyx,cloned_sourceandcloned_targetare not destroyed.How to Fix:
cloned_sourceandcloned_targetin afinallyblock in the callback.More:
value_factoryare not used properly. fixed them at the same time.Analysis:
finallycould correctly decrease the reference countaddressproperty is returned by source_get_address which is a in_memory opeartion (value not cloned, reference count doesn't + 1), that seems to be place where seg fault took placevalue_factory), as AMQPValue will call destroy (reference count - 1) when GC kicks in. the reference count will be incorrect at that time.popmethod in cdef class CompositeValue(AMQPValue)get_body_sequencemethod in cdef class cMessage