[dpdk-dev] member: fix memory leak on error
Checks
Commit Message
>-----Original Message-----
>From: Burakov, Anatoly
>Yep, i can see that now. Didn't think to look inside rte_member_free()
>:/ However, you're creating a race condition there - you're unlocking a
>tailq, and then locking (and unlocking) it again inside
>rte_member_free() - it probably needs _thread_unsafe() functions that
>you can call from behind the lock.
>
>--
Thank you Anatoly,
I realize that rte_member_free does not do anything good here. As a fix, I think the following should work. Is there any other concern?
Thank you!
Comments
On 22-Dec-17 6:33 PM, Wang, Yipeng1 wrote:
>> -----Original Message-----
>> From: Burakov, Anatoly
>> Yep, i can see that now. Didn't think to look inside rte_member_free()
>> :/ However, you're creating a race condition there - you're unlocking a
>> tailq, and then locking (and unlocking) it again inside
>> rte_member_free() - it probably needs _thread_unsafe() functions that
>> you can call from behind the lock.
>>
>> --
>
> Thank you Anatoly,
>
> I realize that rte_member_free does not do anything good here. As a fix, I think the following should work. Is there any other concern?
>
Yes, that should work. Table creation is the last step that can cause an
error, so if we're there, we already know that we couldn't have
allocated it, so there's no need to deallocate those, and simple
rte_free(setsum) should do. I'll submit a v2?
Sure. Thanks Anatoly,
Please go ahead and submit V2.
Thanks
Yipeng
>-----Original Message-----
>From: Burakov, Anatoly
>Sent: Saturday, December 23, 2017 3:55 AM
>To: Wang, Yipeng1 <yipeng1.wang@intel.com>; dev@dpdk.org
>Cc: Gobriel, Sameh <sameh.gobriel@intel.com>
>Subject: Re: [PATCH] member: fix memory leak on error
>
>On 22-Dec-17 6:33 PM, Wang, Yipeng1 wrote:
>>> -----Original Message-----
>>> From: Burakov, Anatoly
>>> Yep, i can see that now. Didn't think to look inside rte_member_free()
>>> :/ However, you're creating a race condition there - you're unlocking a
>>> tailq, and then locking (and unlocking) it again inside
>>> rte_member_free() - it probably needs _thread_unsafe() functions that
>>> you can call from behind the lock.
>>>
>>> --
>>
>> Thank you Anatoly,
>>
>> I realize that rte_member_free does not do anything good here. As a fix, I
>think the following should work. Is there any other concern?
>>
>
>Yes, that should work. Table creation is the last step that can cause an
>error, so if we're there, we already know that we couldn't have
>allocated it, so there's no need to deallocate those, and simple
>rte_free(setsum) should do. I'll submit a v2?
>
>--
>Thanks,
>Anatoly
@@ -192,7 +192,8 @@ rte_member_create(const struct rte_member_parameters *params)
error_unlock_exit:
rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
- rte_member_free(setsum);
+ rte_free(te);
+ rte_free(setsum);
return NULL;
}