[Asterisk-code-review] AST-2017-013: chan skinny: Call pthread detach when sess thr... (asterisk[certified/13.13])

[George Joseph]

George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/7418 )

Change subject: AST-2017-013: chan_skinny: Call pthread_detach when sess threads end
......................................................................

AST-2017-013: chan_skinny: Call pthread_detach when sess threads end

chan_skinny creates a new thread for each new session.  In trying
to be a good cleanup citizen, the threads are joinable and the
unload_module function does a pthread_cancel() and a pthread_join()
on any sessions that are active at that time.  This has an
unintended side effect though. Since you can call pthread_join on a
thread that's already terminated, pthreads keeps the thread's
storage around until you explicitly call pthread_join (or
pthread_detach()).   Since only the module_unload function was
calling pthread_join, and even then only on the ones active at the
tme, the storage for every thread/session ever created sticks
around until asterisk exits.

* A thread can detach itself so the session_destroy() function
  now calls pthread_detach() just before it frees the session
  memory allocation.  The module_unload function still takes care
  of the ones that are still active should the module be unloaded.

ASTERISK-27452
Reported by: Juan Sacco

Change-Id: I9af7268eba14bf76960566f891320f97b974e6dd
---
M channels/chan_skinny.c
1 file changed, 6 insertions(+), 5 deletions(-)

Approvals:
  Jenkins2: Verified
  George Joseph: Looks good to me, approved; Approved for Submit



diff --git a/channels/chan_skinny.c b/channels/chan_skinny.c
index 1054ced..ea0c453 100644
--- a/channels/chan_skinny.c
+++ b/channels/chan_skinny.c
@@ -7414,6 +7414,11 @@
 	}
 	ast_mutex_unlock(&s->lock);
 	ast_mutex_destroy(&s->lock);
+
+	if (s->t != AST_PTHREADT_NULL) {
+		pthread_detach(s->t);
+	}
+
 	ast_free(s);
 }
 
@@ -7499,11 +7504,6 @@
 	int dlen = 0;
 	int eventmessage = 0;
 	struct pollfd fds[1];
-
-	if (!s) {
-		ast_log(LOG_WARNING, "Bad Skinny Session\n");
-		return 0;
-	}
 
 	ast_log(LOG_NOTICE, "Starting Skinny session from %s\n", ast_inet_ntoa(s->sin.sin_addr));
 
@@ -7673,6 +7673,7 @@
 		s->keepalive_timeout_sched = -1;
 
 		if (ast_pthread_create(&s->t, NULL, skinny_session, s)) {
+			s->t = AST_PTHREADT_NULL;
 			destroy_session(s);
 		}
 	}

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

Gerrit-Project: asterisk
Gerrit-Branch: certified/13.13
Gerrit-MessageType: merged
Gerrit-Change-Id: I9af7268eba14bf76960566f891320f97b974e6dd
Gerrit-Change-Number: 7418
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171201/ba6fae9b/attachment.html>