[Asterisk-code-review] named locks: Use ao2 weakproxy find. (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/6711 )

Change subject: named_locks: Use ao2_weakproxy_find.
......................................................................


Patch Set 2: Code-Review-1

(1 comment)

https://gerrit.asterisk.org/#/c/6711/2/main/named_locks.c
File main/named_locks.c:

https://gerrit.asterisk.org/#/c/6711/2/main/named_locks.c@120
PS2, Line 120: 	lock = __ao2_weakproxy_find(named_locks, concat_key, OBJ_SEARCH_KEY | OBJ_NOLOCK,
Uh Oh.

A problem here is if named_locks gets unlocked searching for the proxy/real object we might have two threads creating proxy/real objects for the same named lock.

This is also a pre-existing problem.

Its probably easier to see with the pre-patch code:
* Thread 1 has already obtained the real lock object. (lock ref in weak-proxy and thread 1)
* Thread 2 gets named_locks and finds proxy.
* Thread 2 unlocks named_locks
* Thread 3 gets named locks and finds proxy.
* Thread 1 releases the lock ref, lock ref now 1 and grabs proxy lock, blocks on named_locks because of subscription named_lock_proxy_cb() callback trying to unlink the proxy.
* Thread 3 unlocks named_locks
* Thread 1 completes unlinking the proxy from named_locks and releases proxy lock
* Thread 2 and 3 will now create their own lock object and stick them in named_locks because the proxy is already unlinked from the real obj.

Some slight variations on thread 1 execution ordering could still result in creating multiple lock objects.

Admittedly, three thread interactions are unlikely.  However, they are more likely to happen on a very busy system and leave little to no trace of what happened.



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0ce8a1b7101b6caac6a19f83a89f00eaba1e9d9c
Gerrit-Change-Number: 6711
Gerrit-PatchSet: 2
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Tue, 10 Oct 2017 20:50:50 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171010/58df3ec0/attachment.html>