Bug #94080 for Gearman-XS: Gearman::XS::Client::clear_fn() could free memory immediately.

Right now Gearman::XS::Client::clear_fn() disables any callbacks, but doesn't immediately release them. It could free callbacks immediately, to minimise memory use. Patch attached. As-is this won't apply without the patch in #94079 applied, because it adds more tests to file that that patch creates. Nicholas Clark

commit 9cfa024931c9915da9045849896562b4cea7a837 Author: Nicholas Clark <nick@ccl4.org> Date: Fri Mar 21 11:47:10 2014 +0100 $client->clear_fn() should free callbacks immediately, to minimise memory use. diff --git a/Client.xs b/Client.xs index 8475900..89579d4 100644 --- a/Client.xs +++ b/Client.xs @@ -154,6 +154,21 @@ static SV* _create_client() { return _bless("Gearman::XS::Client", self); } +void _clear_callbacks(gearman_xs_client *self) { + if (self->created_fn) + sv_free(self->created_fn); + if (self->data_fn) + sv_free(self->data_fn); + if (self->complete_fn) + sv_free(self->complete_fn); + if (self->fail_fn) + sv_free(self->fail_fn); + if (self->status_fn) + sv_free(self->status_fn); + if (self->warning_fn) + sv_free(self->warning_fn); +} + MODULE = Gearman::XS::Client PACKAGE = Gearman::XS::Client PROTOTYPES: ENABLE @@ -697,22 +712,18 @@ clear_fn(self) gearman_xs_client *self CODE: gearman_client_clear_fn(self->client); + _clear_callbacks(self); + self->created_fn = NULL; + self->data_fn = NULL; + self->complete_fn = NULL; + self->fail_fn = NULL; + self->status_fn = NULL; + self->warning_fn = NULL; void DESTROY(self) gearman_xs_client *self CODE: gearman_client_free(self->client); - if (self->created_fn) - sv_free(self->created_fn); - if (self->data_fn) - sv_free(self->data_fn); - if (self->complete_fn) - sv_free(self->complete_fn); - if (self->fail_fn) - sv_free(self->fail_fn); - if (self->status_fn) - sv_free(self->status_fn); - if (self->warning_fn) - sv_free(self->warning_fn); + _clear_callbacks(self); Safefree(self); diff --git a/t/81-callbackleak.t b/t/81-callbackleak.t index 84b7d32..fb23fe6 100644 --- a/t/81-callbackleak.t +++ b/t/81-callbackleak.t @@ -90,4 +90,31 @@ undef $client; is($count, 0, 'No callbacks'); +# Clearing should release callbacks immediately +$client = Gearman::XS::Client->new(); + +foreach my $setter (values %setters) { + $client->$setter(gen_callback()); +} + +is($count, $all, "All $all callbacks"); + +$client->clear_fn(); + +is($count, 0, "clear_fn() releases all callbacks"); + +foreach my $setter (values %setters) { + $client->$setter(gen_callback()); +} + +is($count, $all, "All $all callbacks again"); + +$client->clear_fn(); + +is($count, 0, "clear_fn() releases all callbacks again"); + +undef $client; + +is($count, 0, 'No callbacks'); + done_testing();