[Asterisk-code-review] Stasis: Fix potential memory leak of control data. (asterisk[master])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: Stasis: Fix potential memory leak of control data.
......................................................................


Stasis: Fix potential memory leak of control data.

When queuing tasks onto the Stasis control queue, you can pass an
arbitrary data pointer and a function to free that data. All ARI
commands that use the Stasis control queue made the assumption that the
destructor function would be called in all paths, whether the task was
queued successfully or not. However, this was not correct. If a task was
queued onto a control structure that was already completed, the
allocated data would not be freed properly.

This patch corrects this by making sure that all return paths call the
data destructor.

Change-Id: Ibf06522094f8e5c4cce652537dc5d7222b1c4fcb
---
M res/stasis/control.c
1 file changed, 16 insertions(+), 0 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/res/stasis/control.c b/res/stasis/control.c
index 99407df..bae64ec 100644
--- a/res/stasis/control.c
+++ b/res/stasis/control.c
@@ -746,6 +746,14 @@
 	RAII_VAR(struct stasis_app_command *, command, NULL, ao2_cleanup);
 
 	if (control == NULL || control->is_done) {
+		/* If exec_command_on_condition fails, it calls the data_destructor.
+		 * In order to provide consistent behavior, we'll also call the data_destructor
+		 * on this error path. This way, callers never have to call the
+		 * data_destructor themselves.
+		 */
+		if (data_destructor) {
+			data_destructor(data);
+		}
 		return -1;
 	}
 
@@ -771,6 +779,14 @@
 	RAII_VAR(struct stasis_app_command *, command, NULL, ao2_cleanup);
 
 	if (control == NULL || control->is_done) {
+		/* If exec_command fails, it calls the data_destructor. In order to
+		 * provide consistent behavior, we'll also call the data_destructor
+		 * on this error path. This way, callers never have to call the
+		 * data_destructor themselves.
+		 */
+		if (data_destructor) {
+			data_destructor(data);
+		}
 		return -1;
 	}
 

-- 
To view, visit https://gerrit.asterisk.org/2071
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ibf06522094f8e5c4cce652537dc5d7222b1c4fcb
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>