[Asterisk-code-review] pbx: Deadlock between contexts container and context merge l... (asterisk[master])

[Joshua Colp]

Joshua Colp has posted comments on this change.

Change subject: pbx: Deadlock between contexts container and context_merge locks
......................................................................


Patch Set 2: Code-Review-1

(5 comments)

https://gerrit.asterisk.org/#/c/1857/2/main/pbx.c
File main/pbx.c:

Line 724: /*! \brief Subscription for hint change events */
This shouldn't be a separate subscription - each subscription has its own thread so they won't be executed in a serialized fashion in device_state_sub or presence_state_sub. You have to queue it on the topics they are subscribed to and use those subscriptions.


Line 3774: static struct stasis_topic *hint_change_topic;
No specific topic should be needed.


Line 3871: 	/*
         : 	 * Hold off ast_merge_contexts_and_delete, but do it by holding the contexts lock.
         : 	 * If the context_merge_lock is held there is the potential for a deadlock.
         : 	 */
         : 	ast_rdlock_contexts();
Return this function to the exact locking that was done before the original fix. The only change that should be needed to be done is a publish on the device state and presence state topics at the end.


Line 3917: 	presence_state_changed = ((hint->last_presence_state != previous_presence_state) ||
         : 				  strcmp(S_OR(hint->last_presence_subtype, ""), S_OR(previous_subtype, "")) ||
         : 				  strcmp(S_OR(hint->last_presence_message, ""), S_OR(previous_message, "")));
I don't think any changes to the hint for device or presence should be done here or even calculation. It should just be queued up on the device and publish state topics, and left up to them instead.


Line 3944: static void hint_change_cb(void *unused, struct stasis_subscription *sub, struct stasis_message *msg)
Based on my comments above we should reuse device_state_cb and presence_state_cb as much as possible.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f67ce4b4c94b731c23932cc2ba9e0a4a720813d
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-HasComments: Yes