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

[Kevin Harwell]

Kevin Harwell has posted comments on this change.

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


Patch Set 2:

(2 comments)

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

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
I may need more clarification. This is how I understood the problem:

The deadlock occurred because of the context_merge_lock that was here. By placing the context_merge_lock at the start of this function it is possible that the contexts container lock and the merge lock are called out of order.

It should be lock the context_merge_lock first then the container lock. However in this instance the context_merge_lock is potentially locked after the container lock thus creating the possibility of a deadlock.

By only locking the container lock here we can guarantee that the merge/delete code won't run at the same time as this and we also remove the possibility of the deadlock.


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_sta
I had originally thought about that, but it is going to require some refactoring of both the presence and device state callbacks. Which sounds like what you originally had in mind? I was attempting a least intrusive approach, but that may still not be the case. I'll give a stab at re-using those functions and see what the actual changes may look like.


-- 
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-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-HasComments: Yes