summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMark Michelson <mmichelson@digium.com>2015-08-31 15:24:17 -0500
committerMark Michelson <mmichelson@digium.com>2015-08-31 15:24:17 -0500
commit03fe79f29eae42be24589b323a5ef3fa9259158d (patch)
tree82ce2c010667ade4d6f3868bc51a49609eda58d7
parentc2df44ad3ec8685126dd01af6be5bcbd135c1427 (diff)
Fix deadlock on presence state changes.
A deadlock was observed where three threads were competing for different locks: * One thread held the hints lock and was attempting to lock a specific hint. * One thread was holding the specific hint's lock and was attempting to lock the contexts lock * One thread was holding the contexts lock and attempting to lock the hints lock. Clearly the second thread was doing the wrong thing here. The fix for this is to make sure that the hint's lock is not held on presence state changes. Something similar is already done (and commented about) for device state changes. ASTERISK-25362 #close Reported by Mark Michelson Change-Id: I15ec2416b92978a4c0c08273b2d46cb21aff97e2
-rw-r--r--main/pbx.c15
1 files changed, 14 insertions, 1 deletions
diff --git a/main/pbx.c b/main/pbx.c
index f23dff7d2..b34a060ba 100644
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -11978,10 +11978,11 @@ static void presence_state_cb(void *unused, struct stasis_subscription *sub, str
struct ast_state_cb *state_cb;
const char *app;
char *parse;
- SCOPED_AO2LOCK(lock, hint);
+ ao2_lock(hint);
if (!hint->exten) {
/* The extension has already been destroyed */
+ ao2_unlock(hint);
continue;
}
@@ -11989,16 +11990,19 @@ static void presence_state_cb(void *unused, struct stasis_subscription *sub, str
app = ast_get_extension_app(hint->exten);
if (ast_strlen_zero(app)) {
/* The hint does not monitor presence at all. */
+ ao2_unlock(hint);
continue;
}
ast_str_set(&hint_app, 0, "%s", app);
parse = parse_hint_presence(hint_app);
if (ast_strlen_zero(parse)) {
+ ao2_unlock(hint);
continue;
}
if (strcasecmp(parse, presence_state->provider)) {
/* The hint does not monitor the presence provider. */
+ ao2_unlock(hint);
continue;
}
@@ -12019,6 +12023,7 @@ static void presence_state_cb(void *unused, struct stasis_subscription *sub, str
((hint->last_presence_message && presence_state->message && !strcmp(hint->last_presence_message, presence_state->message)) || (!hint->last_presence_message && !presence_state->message))) {
/* this update is the same as the last, do nothing */
+ ao2_unlock(hint);
continue;
}
@@ -12028,6 +12033,14 @@ static void presence_state_cb(void *unused, struct stasis_subscription *sub, str
hint->last_presence_state = presence_state->state;
hint->last_presence_subtype = presence_state->subtype ? ast_strdup(presence_state->subtype) : NULL;
hint->last_presence_message = presence_state->message ? ast_strdup(presence_state->message) : NULL;
+ /*
+ * (Copied from device_state_cb)
+ *
+ * NOTE: We cannot hold any locks while notifying
+ * the watchers without causing a deadlock.
+ * (conlock, hints, and hint)
+ */
+ ao2_unlock(hint);
/* For general callbacks */
cb_iter = ao2_iterator_init(statecbs, 0);