Fix disjoint_pool_free() and proxy_free() #1157
Conversation
| return ret; | ||
| } | ||
|
|
||
| if (pool != umfPoolGetPoolPriv(allocInfo.pool)) { |
There was a problem hiding this comment.
Why do we need this check? It breaks encapsulation because now pool implementation calls umfPoolGetPoolPriv.
There was a problem hiding this comment.
This check prevents from freeing a pointer from a different pool.
There was a problem hiding this comment.
@vinser52 It is needed by #1143
See #1143 (comment)
There was a problem hiding this comment.
I am not sure we should expose UMF internals to the pool implementation.
Furthermore, this solution works only for UMF's pools. If someone implements their pool what he/she should do in that case?
There was a problem hiding this comment.
This PR was replaced with #1161 - please review this new PR instead.
|
The name of this PR is misleading because it does not fix anything, it just introduces an additional check. But do we really need this check? |
|
Fix disjoint_pool_free() and proxy_free() - error out on pool mismatch, when we try to free the pointer belonging to a different pool. Signed-off-by: Lukasz Dorau <[email protected]>
ldorau
force-pushed
the
Fix_disjoint_pool_free_and_proxy_free
branch
from
5300876 to
32175d9
Compare
it is the ill-formed client. |
|
I'm not sure if adding this check in the pool is a good idea. As I understand, all other pools will NOT return an error if we try to free memory from a different pool (which is inconsistent). And even if we added this check to proxy_pool, jemalloc_pool, what about custom user pools? |
This PR was replaced with #1161 - please review this new PR instead. |
4 participants
Description
Fix disjoint_pool_free() and proxy_free() - error out on pool mismatch, when we try to free the pointer
belonging to a different pool.
Checklist