[Linux-ha-dev] [PATCH] High: ccm: fix a memory leak when a client exits

[Keisuke MORI]

Hi,

The attached patch will fix a memory leak in ccm that occurs whenever a ccm
client disconnect.

It would not affect to most of the installations because only crmd and cib
are the client, but if you run any ccm client such as crm_node command
periodically, ccm will increase its memory consumption.

The valgrind outputs are also attached as the evidence of the leakage and
the fix by the patch;
The results are taken after crm_node command is executed 100 times.

There still exists some definitely / indirectly / possibly lost , but as
long as I've investigated they are all allocated only at the invocation
time and not considered as a leak. Double checks are welcome.

Thanks,

[Lars Ellenberg]

Post by Keisuke MORI
Hi,
The attached patch will fix a memory leak in ccm that occurs whenever a ccm
client disconnect.

Thank you.

This may introduce double free for client_delete_all() now?

All this aparently useless indirection seems to be from a time
when client_destroy explicitly called into a ->ops->destroy "virtual
function". Which it no longer does.

So I think dropping the explicit calls to client_destroy, as well as
the other then useless indirection functions, but instead do a
g_hash_table_new_full with g_free in client_init would be the way to go.

Could you have a look?

Post by Keisuke MORI
It would not affect to most of the installations because only crmd and cib
are the client, but if you run any ccm client such as crm_node command
periodically, ccm will increase its memory consumption.
The valgrind outputs are also attached as the evidence of the leakage and
the fix by the patch;
The results are taken after crm_node command is executed 100 times.
There still exists some definitely / indirectly / possibly lost , but as
long as I've investigated they are all allocated only at the invocation
time and not considered as a leak. Double checks are welcome.
Thanks,
--
Keisuke MORI

Cheers,
Lars

[Keisuke MORI]

Hi,

Post by Lars Ellenberg

Post by Keisuke MORI
Hi,
The attached patch will fix a memory leak in ccm that occurs whenever a ccm
client disconnect.

Thank you.
This may introduce double free for client_delete_all() now?

No, I do not think it does.
When an individual client exits, client_delete() removes the ipc
object from ccm_hashclient and hence
client_detete_all() will never call client_destroy() for the same ipc
object again.
The valgrind result did not complain regarding to this either.

Am I missing your point?

Post by Lars Ellenberg
All this aparently useless indirection seems to be from a time
when client_destroy explicitly called into a ->ops->destroy "virtual
function". Which it no longer does.
So I think dropping the explicit calls to client_destroy, as well as
the other then useless indirection functions, but instead do a
g_hash_table_new_full with g_free in client_init would be the way to go.

It might be doable, but I do not think it is necessary to rewrite the
code for fixing this issue.

Thanks,

Post by Lars Ellenberg
Could you have a look?

Post by Keisuke MORI
It would not affect to most of the installations because only crmd and cib
are the client, but if you run any ccm client such as crm_node command
periodically, ccm will increase its memory consumption.
The valgrind outputs are also attached as the evidence of the leakage and
the fix by the patch;
The results are taken after crm_node command is executed 100 times.
There still exists some definitely / indirectly / possibly lost , but as
long as I've investigated they are all allocated only at the invocation
time and not considered as a leak. Double checks are welcome.
Thanks,
--
Keisuke MORI

Cheers,
Lars
--
: Lars Ellenberg
: LINBIT | Your Way to High Availability
: DRBD/HA support and consulting http://www.linbit.com
DRBD® and LINBIT® are registered trademarks of LINBIT, Austria.
_______________________________________________________
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/