[Asterisk-code-review] manager: Use separate lock for session event notification. (...asterisk[13])

Sean Bright asteriskteam at digium.com
Tue Mar 26 08:31:19 CDT 2019


Sean Bright has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/11179 )

Change subject: manager: Use separate lock for session event notification.
......................................................................


Patch Set 1:

(2 comments)

My understanding (which could very well be incorrect) is that if you are protecting mutable state with a lock, you need to consistently use the same lock. Not -1ing, I just wanted it to be seen.

https://gerrit.asterisk.org/#/c/11179/1/main/manager.c 
File main/manager.c:

https://gerrit.asterisk.org/#/c/11179/1/main/manager.c@4158 
PS1, Line 4158: 
              : 	if (s->session->waiting_thread != AST_PTHREADT_NULL) {
              : 		pthread_kill(s->session->waiting_thread, SIGURG);
              : 	}
waiting_thread isn't protected with notify_lock here


https://gerrit.asterisk.org/#/c/11179/1/main/manager.c@7907 
PS1, Line 7907: 
              : 			if (session->waiting_thread != AST_PTHREADT_NULL) {
              : 				pthread_kill(session->waiting_thread, SIGURG);
              : 			}
waiting_thread isn't protected with notify_lock here



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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: Ifbcac007faca9ad0231640f5e82a6ca9228f261b
Gerrit-Change-Number: 11179
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
Gerrit-Comment-Date: Tue, 26 Mar 2019 13:31:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190326/fc7e8b78/attachment.html>


More information about the asterisk-code-review mailing list