[Asterisk-code-review] app_signal: Add signaling applications (asterisk[master])

Joshua Colp asteriskteam at digium.com
Thu Dec 22 04:25:59 CST 2022


Attention is currently required from: N A, Mark Murawski.

Joshua Colp has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/17786 )

Change subject: app_signal: Add signaling applications
......................................................................


Patch Set 4: Code-Review-1

(4 comments)

File apps/app_signal.c:

https://gerrit.asterisk.org/c/asterisk/+/17786/comment/900e79f3_24009fef 
PS4, Line 160: 	}
This hasn't called ast_mutex_destroy


https://gerrit.asterisk.org/c/asterisk/+/17786/comment/b56d8018_c6639b90 
PS4, Line 171: 		ast_mutex_lock(&s->lock);
Why lock and then unlock?


https://gerrit.asterisk.org/c/asterisk/+/17786/comment/5e352a34_db01c517 
PS4, Line 278: 		remove_signal(signame);
This has a race condition where another thread could be finding this signal, while it is being removed. The signals list is not locked during this. Due to the fact that watchers is also not touched as part of the get process, that other thread could end up using freed memory.


https://gerrit.asterisk.org/c/asterisk/+/17786/comment/e5c363e0_c79522bb 
PS4, Line 422: 		ast_log(LOG_WARNING, "One or more signals is currently in use. Unload failed.\n");
What does this actually mean for a user? You've removed all of the signals from the list, so will those just go away - will they stick around?



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/17786
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: Ic34439de3d60f8609357666a465c354d81f5fef3
Gerrit-Change-Number: 17786
Gerrit-PatchSet: 4
Gerrit-Owner: N A <asterisk at phreaknet.org>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Mark Murawski <markm at intellasoft.net>
Gerrit-Reviewer: N A <asterisk at phreaknet.org>
Gerrit-Attention: N A <asterisk at phreaknet.org>
Gerrit-Attention: Mark Murawski <markm at intellasoft.net>
Gerrit-Comment-Date: Thu, 22 Dec 2022 10:25:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20221222/86e9a49a/attachment-0001.html>


More information about the asterisk-code-review mailing list