There was a fairly common case where the allocated svc_action_t object was not being freed. On an asynchronous services action that was not recurring, this object got leaked. This patch resolves this by adjusting the logic to always free this once the action is completed.
Signed-off-by: Russell Bryant russell@russellbryant.net --- src/lib/services_linux.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/lib/services_linux.c b/src/lib/services_linux.c index 1cc2c4d..632d881 100644 --- a/src/lib/services_linux.c +++ b/src/lib/services_linux.c @@ -181,6 +181,7 @@ operation_finished(mainloop_child_t *p, int status, int signo, int exitcode) char *next = NULL; char *offset = NULL; svc_action_t *op = p->privatedata; + int recurring = 0;
p->privatedata = NULL; op->status = LRM_OP_DONE; @@ -234,20 +235,23 @@ operation_finished(mainloop_child_t *p, int status, int signo, int exitcode) }
if (op->interval && op->opaque->cancel == FALSE) { + recurring = 1; op->opaque->repeat_timer = g_timeout_add(op->interval, recurring_action_timer, (void *) op); }
op->pid = 0; + if (op->opaque->callback) { - /* Callback might call cancel which would result in the message - * being free'd - * Do not access 'op' after this line - */ op->opaque->callback(op); + }
- } else if (op->opaque->repeat_timer == 0) { + if (!recurring) { + /* + * If this is a recurring action, do not free explicitly. + * It will get freed whenever the action gets cancelled. + */ services_action_free(op); } }
At one point I had planned to keep the operation history around for querying. This is probably a remnant of that. Thanks for spotting it.
ack
On Mon, Jun 20, 2011 at 5:17 PM, Russell Bryant russell@russellbryant.net wrote:
There was a fairly common case where the allocated svc_action_t object was not being freed. On an asynchronous services action that was not recurring, this object got leaked. This patch resolves this by adjusting the logic to always free this once the action is completed.
Signed-off-by: Russell Bryant russell@russellbryant.net
src/lib/services_linux.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/lib/services_linux.c b/src/lib/services_linux.c index 1cc2c4d..632d881 100644 --- a/src/lib/services_linux.c +++ b/src/lib/services_linux.c @@ -181,6 +181,7 @@ operation_finished(mainloop_child_t *p, int status, int signo, int exitcode) char *next = NULL; char *offset = NULL; svc_action_t *op = p->privatedata;
- int recurring = 0;
p->privatedata = NULL; op->status = LRM_OP_DONE; @@ -234,20 +235,23 @@ operation_finished(mainloop_child_t *p, int status, int signo, int exitcode) }
if (op->interval && op->opaque->cancel == FALSE) {
- recurring = 1;
op->opaque->repeat_timer = g_timeout_add(op->interval, recurring_action_timer, (void *) op); }
op->pid = 0;
if (op->opaque->callback) {
- /* Callback might call cancel which would result in the message
- * being free'd
- * Do not access 'op' after this line
- */
op->opaque->callback(op);
- }
- } else if (op->opaque->repeat_timer == 0) {
- if (!recurring) {
- /*
- * If this is a recurring action, do not free explicitly.
- * It will get freed whenever the action gets cancelled.
- */
services_action_free(op); } } -- 1.7.5.4
Matahari mailing list Matahari@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/matahari
matahari@lists.fedorahosted.org