Hi Minh,
AMFA currently does not remember the allocated memory. It relies on the
application always to free the memory. In saAmfFinalize() also, it does
not free the memory. I think AMFA should remember memory by associating
it with handle because process which is starting PG tracking may not be
a component and may not call free()/Free_4() and just relies on
saAmfFinalize() call to release all the resources.
I think as of now please go ahead with your suggested solution. From
finalize perspective this is a defect and applicable to all the
branches. So please raise a ticket for that.
Thanks,
Praveen
On 22-Aug-16 12:08 PM, minh chau wrote:
> Hi Praveen,
>
> The case you just mentioned is still in callback context, so Agent can
> help application to release the allocated notification. But still
> another case:
>
> + SaAmfProtectionGroupNotificationBufferT buff;
> + buff.notification = NULL;
> + rc = saAmfProtectionGroupTrack_4(my_amf_hdl, &track_csi,
> SA_TRACK_CURRENT, &buff);
> + if (rc != SA_AIS_OK) {
> + syslog(LOG_ERR, "saAmfProtectionGroupTrack FAILED - %u", rc);
> + goto done;
> + }
>
> In this case Agent has to allocate notification but it's not in Agent's
> context.
> Application has to call API Free_4(buff.notification) to free up
> notification.
> In order to iterate to free longDn(s) inside Free_4(), Agent has to
> memorize a list numberOfItems for every single call as above Track_4(),
> or Agent can add sentinel element to the allocated notification.
>
> Thanks,
> Minh
>
> On 22/08/16 15:34, praveen malviya wrote:
>> Hi,
>> The callback looks like this:
>> typedef void
>> (*SaAmfProtectionGroupTrackCallbackT_4)(
>> const SaNameT *csiName,
>> SaAmfProtectionGroupNotificationBufferT_4 *notificationBuffer,
>> SaUint32T numberOfMembers,
>> SaAisErrorT error);
>>
>> Inside this callback, application is supposed to call
>> saAmfProtectionGroupNotificationFree_4(). So agent must be able to
>> deduce this information as SaAmfProtectionGroupNotificationBufferT_4
>> contains numberOfItems and also numberOfMembers is available from
>> callback.
>> Since B.04.01 APIs are not fully implemented, agent copies from old
>> type of structure to new type in ava_cpy_protection_group_ntf().
>>
>>
>> Thanks,
>> Praveen
>>
>> On 22-Aug-16 10:51 AM, minh chau wrote:
>>> Hi Praveen,
>>>
>>> The problem with B.04.01 is the API:
>>> saAmfProtectionGroupNotificationFree_4(SaAmfHandleT hdl,
>>> SaAmfProtectionGroupNotificationT_4 *notification) does not have
>>> numberOfItems.
>>> Agent does not know how many element in *notification, each of element
>>> can hide a longDn inside it.
>>>
>>> Thanks,
>>> Minh
>>>
>>>
>>> On 22/08/16 15:04, praveen malviya wrote:
>>>> Hi Minh,
>>>>
>>>> SaAmfProtectionGroupNotificationBufferT_4() contains numberOfItems to
>>>> iterate over. In case of B.04.01, it should be simple as agent can
>>>> call direclty osaf_extended_name_free() during iteration inside
>>>> saAmfProtectionGroupNotificationFree_4(). So I think, only a for loop
>>>> which will iterate over numberOfItems is required.
>>>>
>>>> Problem was in B.01.01 case, where application will have to iterate
>>>> and free the memory. For this, Long has already suggested and that
>>>> needs to be documented.
>>>>
>>>>
>>>> Thanks,
>>>> Praveen
>>>>
>>>>
>>>> On 20-Aug-16 2:22 PM, minh chau wrote:
>>>>> Hi Long, Praveen,
>>>>>
>>>>> Regarding this TODO
>>>>> + if(notification) {
>>>>> + // TODO (minhchau): memleak if notification is an array
>>>>> + osaf_extended_name_free(¬ification->member.compName);
>>>>> free(notification);
>>>>> + }
>>>>>
>>>>> Client currently uses saAmfProtectionGroupNotificationFree_4(handle,
>>>>> buff->notification) to free the notification in buffer.
>>>>> If @buff->notification is a list of shortDn only, that should work as
>>>>> before, as agent will call this inside
>>>>> saAmfProtectionGroupNotificationFree_4
>>>>>
>>>>> /* free memory */
>>>>> if(notification)
>>>>> free(notification);
>>>>>
>>>>> It will cause memory leak if @buff->notification contains a list of
>>>>> longDN notifications.
>>>>> The leak is longDn of compName in each notification after the the
>>>>> first
>>>>> one in the array @buff->notification.
>>>>>
>>>>> Agent can add a sentinel element when agent allocates
>>>>> @buff->notification, set this last element as NULL
>>>>> In Free() API, agent could iterate and free longDn in each element of
>>>>> array @buff->notification until agent reaches NULL element.
>>>>>
>>>>> Do you think it could work?
>>>>
>>>>>
>>>>> Thanks,
>>>>> Minh
>>>>>
>>>>> On 19/08/16 21:13, Long Nguyen wrote:
>>>>>> Hi Praveen,
>>>>>>
>>>>>> Please see my answers marked with [Long].
>>>>>>
>>>>>> Best regards,
>>>>>> Long Nguyen.
>>>>>>
>>>>>> On 8/19/2016 6:01 PM, praveen malviya wrote:
>>>>>>> Hi Long,
>>>>>>>
>>>>>>> I see one problem if B.01.01 application frees the memory in pg
>>>>>>> tracking callback.
>>>>>>> Please see inline.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Praveen
>>>>>>> On 19-Aug-16 12:00 PM, Long HB Nguyen wrote:
>>>>>>>> osaf/libs/agents/saf/amfa/amf_agent.cc | 1 +
>>>>>>>> osaf/libs/agents/saf/amfa/ava_hdl.cc | 2 --
>>>>>>>> 2 files changed, 1 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>>
>>>>>>>> diff --git a/osaf/libs/agents/saf/amfa/amf_agent.cc
>>>>>>>> b/osaf/libs/agents/saf/amfa/amf_agent.cc
>>>>>>>> --- a/osaf/libs/agents/saf/amfa/amf_agent.cc
>>>>>>>> +++ b/osaf/libs/agents/saf/amfa/amf_agent.cc
>>>>>>>> @@ -2450,6 +2450,7 @@ SaAisErrorT AmfAgent::ProtectionGroupTra
>>>>>>>> ava_cpy_protection_group_ntf(buf->notification,
>>>>>>>> rsp_buf->notification,
>>>>>>>> buf->numberOfItems,
>>>>>>>> SA_AMF_HARS_READY_FOR_ASSIGNMENT);
>>>>>>>> rc = SA_AIS_ERR_NO_SPACE;
>>>>>>>> + buf->numberOfItems = rsp_buf->numberOfItems;
>>>>>>>> }
>>>>>>>> } else { /* if(create_memory == false) */
>>>>>>>>
>>>>>>>> diff --git a/osaf/libs/agents/saf/amfa/ava_hdl.cc
>>>>>>>> b/osaf/libs/agents/saf/amfa/ava_hdl.cc
>>>>>>>> --- a/osaf/libs/agents/saf/amfa/ava_hdl.cc
>>>>>>>> +++ b/osaf/libs/agents/saf/amfa/ava_hdl.cc
>>>>>>>> @@ -697,7 +697,6 @@ uint32_t ava_hdl_cbk_rec_prc(AVSV_AMF_CB
>>>>>>>> ((SaAmfCallbacksT_4*)reg_cbk)->saAmfProtectionGroupTrackCallback(&pg_track->csi_name,
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> &buf,
>>>>>>>> pg_track->mem_num, pg_track->err);
>>>>>>>> - free(buf.notification);
>>>>>>>> } else {
>>>>>>>> pg_track->err = SA_AIS_ERR_NO_MEMORY;
>>>>>>>> LOG_CR("Notification is NULL: Invoking
>>>>>>>> PGTrack Callback with error SA_AIS_ERR_NO_MEMORY");
>>>>>>>> @@ -740,7 +739,6 @@ uint32_t ava_hdl_cbk_rec_prc(AVSV_AMF_CB
>>>>>>>> ((SaAmfCallbacksT
>>>>>>>> *)reg_cbk)->saAmfProtectionGroupTrackCallback(&pg_track->csi_name,
>>>>>>>> &buf,
>>>>>>>> pg_track->mem_num, pg_track->err);
>>>>>>>> - free(buf.notification);
>>>>>>> For B.04.01 API, saAmfProtectionGroupNotificationFree_4() is taking
>>>>>>> care of freeing any extended name. For
>>>>>>> saAmfProtectionGroupNotificationFree(), it is the application's
>>>>>>> responsibility to free the memory. But how it will free any extended
>>>>>>> name.
>>>>>>> I think there is no API equivalent to osaf_extended_name_free() for
>>>>>>> application. Is there any way?
>>>>>>> [Long] I think we can do somethings like in applications:
>>>>>>> if (strlen(saAisNameBorrow(buff.notification[i].member.comp_name)) >
>>>>>>> SA_MAX_UNEXTENDED_NAME_LENGTH)
>>>>>>> free(saAisNameBorrow(buff.notification[i].member.comp_name));
>>>>>>>
>>>>>>> Otherwise we can document it that:
>>>>>>> -if compName is not long dn in the notification buffer, then
>>>>>>> application has to free the memory.This will provide backward
>>>>>>> compatibility and spec compliance.
>>>>>>>
>>>>>>> -if compName is longdn in notification then application should not
>>>>>>> free the memory. Agent will free the memory after callback is
>>>>>>> completed. So any B.01.01 application adapting to long dn will take
>>>>>>> care of this when modifying the application. In that case we need to
>>>>>>> do something like this :
>>>>>>> for i in notificationBuffer->notification[i]
>>>>>>> if
>>>>>>> (osaf_is_an_extended_name(notificationBuffer->notification[i].member.compName)){
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> longdn_found = true;
>>>>>>> osaf_extended_name_free();
>>>>>>> }
>>>>>>> }
>>>>>>> if(longdn_found)
>>>>>>> free(buf.notification)
>>>>>>>
>>>>>>>
>>>>>>> Also there is a Todo in amf_agent.cc " // TODO (minhchau):
>>>>>>> memleak if
>>>>>>> notification is an array".
>>>>>>> [Long] Thanks, I will check it.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Praveen
>>>>>>>
>>>>>>>> } else {
>>>>>>>> pg_track->err = SA_AIS_ERR_NO_MEMORY;
>>>>>>>> LOG_CR("Notification is NULL: Invoking
>>>>>>>> PGTrack Callback with error SA_AIS_ERR_NO_MEMORY");
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
