channel locking: Add locking for channel snapshot creation

Original commit message by mmichelson (asterisk 12 r403311):

"This adds channel locks around calls to create channel snapshots as well
as other functions which operate on a channel and then end up
creating a channel snapshot. Functions that expect the channel to be
locked prior to being called have had their documentation updated to
indicate such."

The above was initially committed and then reverted at r403398.  The problem
was found to be in core_local.c in the publish_local_bridge_message function.
The ast_unreal_lock_all function locks and adds a reference to the returned
channels and while they were being unlocked they were not being unreffed when
no longer needed.  Fixed by unreffing the channels.

Also in bridge.c a lock was obtained on "other->chan", but then an attempt was
made to unlock "other" and not the previously locked channel.  Fixed by
unlocking "other->chan"

(closes issue ASTERISK-22709)
Reported by: John Bigelow
........

Merged revisions 404237 from http://svn.asterisk.org/svn/asterisk/branches/12


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@404260 65c4cc65-6c06-0410-ace0-fbb531ad65f3

5 files changed, 79 insertions, 6 deletions

diff --git a/include/asterisk/aoc.h b/include/asterisk/aoc.h
index 727362c1f..171fbb300 100644
--- a/include/asterisk/aoc.h
+++ b/include/asterisk/aoc.h
@@ -497,7 +497,10 @@ int ast_aoc_s_add_special_arrangement(struct ast_aoc_decoded *decoded,
  */
 int ast_aoc_decoded2str(const struct ast_aoc_decoded *decoded, struct ast_str **msg);
 
-/*! \brief generate AOC manager event for an AOC-S, AOC-D, or AOC-E msg */
+/*!
+ * \brief generate AOC manager event for an AOC-S, AOC-D, or AOC-E msg
+ * \pre chan is locked
+ */
 int ast_aoc_manager_event(const struct ast_aoc_decoded *decoded, struct ast_channel *chan);
 
 /*! \brief get the message type, AOC-D, AOC-E, or AOC Request */
diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index 51530a82b..80ed7458c 100644
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -1538,8 +1538,7 @@ int ast_channel_cmpwhentohangup_tv(struct ast_channel *chan, struct timeval offs
  * \details
  * This function sets the absolute time out on a channel (when to hang up).
  *
- * \note This function does not require that the channel is locked before
- *       calling it.
+ * \pre chan is locked
  *
  * \return Nothing
  * \sa ast_channel_setwhentohangup_tv()
@@ -1555,8 +1554,7 @@ void ast_channel_setwhentohangup(struct ast_channel *chan, time_t offset) __attr
  *
  * This function sets the absolute time out on a channel (when to hang up).
  *
- * \note This function does not require that the channel is locked before
- * calling it.
+ * \pre chan is locked
  *
  * \return Nothing
  * \since 1.6.1
@@ -2340,6 +2338,8 @@ void ast_channel_inherit_variables(const struct ast_channel *parent, struct ast_
  * \param chan the channel
  * \param vars a linked list of variables
  *
+ * \pre chan is locked
+ *
  * \details
  * Variable names can be for a regular channel variable or a dialplan function
  * that has the ability to be written to.
@@ -3806,6 +3806,15 @@ void ast_channel_name_set(struct ast_channel *chan, const char *name);
 	void ast_channel_##field##_build_va(struct ast_channel *chan, const char *fmt, va_list ap) __attribute__((format(printf, 2, 0))); \
 	void ast_channel_##field##_build(struct ast_channel *chan, const char *fmt, ...) __attribute__((format(printf, 2, 3)))
 
+/*!
+ * The following string fields result in channel snapshot creation and
+ * should have the channel locked when called:
+ *
+ * \li language
+ * \li accountcode
+ * \li peeracccount
+ * \li linkedid
+ */
 DECLARE_STRINGFIELD_SETTERS_FOR(name);
 DECLARE_STRINGFIELD_SETTERS_FOR(language);
 DECLARE_STRINGFIELD_SETTERS_FOR(musicclass);
@@ -3857,6 +3866,10 @@ void ast_channel_sending_dtmf_digit_set(struct ast_channel *chan, char value);
 struct timeval ast_channel_sending_dtmf_tv(const struct ast_channel *chan);
 void ast_channel_sending_dtmf_tv_set(struct ast_channel *chan, struct timeval value);
 enum ama_flags ast_channel_amaflags(const struct ast_channel *chan);
+
+/*!
+ * \pre chan is locked
+ */
 void ast_channel_amaflags_set(struct ast_channel *chan, enum ama_flags value);
 int ast_channel_epfd(const struct ast_channel *chan);
 void ast_channel_epfd_set(struct ast_channel *chan, int value);
@@ -3940,6 +3953,10 @@ enum ast_channel_adsicpe ast_channel_adsicpe(const struct ast_channel *chan);
 void ast_channel_adsicpe_set(struct ast_channel *chan, enum ast_channel_adsicpe value);
 enum ast_channel_state ast_channel_state(const struct ast_channel *chan);
 struct ast_callid *ast_channel_callid(const struct ast_channel *chan);
+
+/*!
+ * \pre chan is locked
+ */
 void ast_channel_callid_set(struct ast_channel *chan, struct ast_callid *value);
 
 /* XXX Internal use only, make sure to move later */
@@ -3980,6 +3997,10 @@ void ast_channel_connected_set(struct ast_channel *chan, struct ast_party_connec
 void ast_channel_dialed_set(struct ast_channel *chan, struct ast_party_dialed *value);
 void ast_channel_redirecting_set(struct ast_channel *chan, struct ast_party_redirecting *value);
 void ast_channel_dtmf_tv_set(struct ast_channel *chan, struct timeval *value);
+
+/*!
+ * \pre chan is locked
+ */
 void ast_channel_whentohangup_set(struct ast_channel *chan, struct timeval *value);
 void ast_channel_varshead_set(struct ast_channel *chan, struct varshead *value);
 struct timeval ast_channel_creationtime(struct ast_channel *chan);
@@ -3995,8 +4016,14 @@ struct ast_readq_list *ast_channel_readq(struct ast_channel *chan);
 
 /* Typedef accessors */
 ast_group_t ast_channel_callgroup(const struct ast_channel *chan);
+/*!
+ * \pre chan is locked
+ */
 void ast_channel_callgroup_set(struct ast_channel *chan, ast_group_t value);
 ast_group_t ast_channel_pickupgroup(const struct ast_channel *chan);
+/*!
+ * \pre chan is locked
+ */
 void ast_channel_pickupgroup_set(struct ast_channel *chan, ast_group_t value);
 struct ast_namedgroups *ast_channel_named_callgroups(const struct ast_channel *chan);
 void ast_channel_named_callgroups_set(struct ast_channel *chan, struct ast_namedgroups *value);
@@ -4043,6 +4070,9 @@ ast_timing_func_t ast_channel_timingfunc(const struct ast_channel *chan);
 void ast_channel_timingfunc_set(struct ast_channel *chan, ast_timing_func_t value);
 
 struct ast_bridge *ast_channel_internal_bridge(const struct ast_channel *chan);
+/*!
+ * \pre chan is locked
+ */
 void ast_channel_internal_bridge_set(struct ast_channel *chan, struct ast_bridge *value);
 
 struct ast_bridge_channel *ast_channel_internal_bridge_channel(const struct ast_channel *chan);
diff --git a/include/asterisk/channelstate.h b/include/asterisk/channelstate.h
index f5f7392dd..08f908256 100644
--- a/include/asterisk/channelstate.h
+++ b/include/asterisk/channelstate.h
@@ -47,7 +47,10 @@ enum ast_channel_state {
 	AST_STATE_MUTE = (1 << 16),	/*!< Do not transmit voice data */
 };
 
-/*! \brief Change the state of a channel */
+/*!
+ * \brief Change the state of a channel
+ * \pre chan is locked
+ */
 int ast_setstate(struct ast_channel *chan, enum ast_channel_state);
 
 #endif /* __AST_CHANNELSTATE_H__ */
diff --git a/include/asterisk/stasis_bridges.h b/include/asterisk/stasis_bridges.h
index 2d15a3332..72bf59948 100644
--- a/include/asterisk/stasis_bridges.h
+++ b/include/asterisk/stasis_bridges.h
@@ -203,6 +203,9 @@ struct stasis_message_type *ast_channel_left_bridge_type(void);
  * should also be treated as immutable and not modified after it is put into the
  * message.
  *
+ * \pre bridge is locked.
+ * \pre No channels are locked.
+ *
  * \param bridge Channel blob is associated with, or NULL for global/all bridges.
  * \param blob JSON object representing the data.
  * \return \ref ast_bridge_blob message.
@@ -217,6 +220,9 @@ struct stasis_message *ast_bridge_blob_create(struct stasis_message_type *type,
  * \since 12
  * \brief Publish a bridge channel enter event
  *
+ * \pre bridge is locked.
+ * \pre No channels are locked.
+ *
  * \param bridge The bridge a channel entered
  * \param chan The channel that entered the bridge
  * \param swap The channel being swapped out of the bridge
@@ -228,6 +234,9 @@ void ast_bridge_publish_enter(struct ast_bridge *bridge, struct ast_channel *cha
  * \since 12
  * \brief Publish a bridge channel leave event
  *
+ * \pre bridge is locked.
+ * \pre No channels are locked.
+ *
  * \param bridge The bridge a channel left
  * \param chan The channel that left the bridge
  */
@@ -272,6 +281,8 @@ struct stasis_message_type *ast_blind_transfer_type(void);
 /*!
  * \brief Publish a blind transfer event
  *
+ * \pre No channels or bridges are locked
+ *
  * \param is_external Whether the blind transfer was initiated externally (e.g. via AMI or native protocol)
  * \param result The success or failure of the transfer
  * \param to_transferee The bridge between the transferer and transferee plus the transferer channel
@@ -335,6 +346,8 @@ struct stasis_message_type *ast_attended_transfer_type(void);
  * Publish an \ref ast_attended_transfer_message with the dest_type set to
  * \c AST_ATTENDED_TRANSFER_DEST_FAIL.
  *
+ * \pre No channels or bridges are locked
+ *
  * \param is_external Indicates if the transfer was initiated externally
  * \param result The result of the transfer. Will always be a type of failure.
  * \param transferee The bridge between the transferer and transferees as well as the transferer channel from that bridge
@@ -356,6 +369,8 @@ void ast_bridge_publish_attended_transfer_fail(int is_external, enum ast_transfe
  *
  * In either case, two bridges enter, one leaves.
  *
+ * \pre No channels or bridges are locked
+ *
  * \param is_external Indicates if the transfer was initiated externally
  * \param result The result of the transfer.
  * \param transferee The bridge between the transferer and transferees as well as the transferer channel from that bridge
@@ -375,6 +390,8 @@ void ast_bridge_publish_attended_transfer_bridge_merge(int is_external, enum ast
  * this results from merging two bridges together. The difference is that a
  * transferer channel survives the bridge merge
  *
+ * \pre No channels or bridges are locked
+ *
  * \param is_external Indicates if the transfer was initiated externally
  * \param result The result of the transfer.
  * \param transferee The bridge between the transferer and transferees as well as the transferer channel from that bridge
@@ -396,6 +413,8 @@ void ast_bridge_publish_attended_transfer_threeway(int is_external, enum ast_tra
  * \li A transferee channel leaving a bridge to run an app
  * \li A bridge of transferees running an app (via a local channel)
  *
+ * \pre No channels or bridges are locked
+ *
  * \param is_external Indicates if the transfer was initiated externally
  * \param result The result of the transfer.
  * \param transferee The bridge between the transferer and transferees as well as the transferer channel from that bridge
@@ -419,6 +438,8 @@ void ast_bridge_publish_attended_transfer_app(int is_external, enum ast_transfer
  * When this type of transfer occurs, the two bridges continue to exist after the
  * transfer and a local channel is used to link the two bridges together.
  *
+ * \pre No channels or bridges are locked
+ *
  * \param is_external Indicates if the transfer was initiated externally
  * \param result The result of the transfer.
  * \param transferee The bridge between the transferer and transferees as well as the transferer channel from that bridge
diff --git a/include/asterisk/stasis_channels.h b/include/asterisk/stasis_channels.h
index 8c27803df..9c64c1c49 100644
--- a/include/asterisk/stasis_channels.h
+++ b/include/asterisk/stasis_channels.h
@@ -153,6 +153,8 @@ struct stasis_message_type *ast_channel_snapshot_type(void);
  * \brief Generate a snapshot of the channel state. This is an ao2 object, so
  * ao2_cleanup() to deallocate.
  *
+ * \pre chan is locked
+ *
  * \param chan The channel from which to generate a snapshot
  *
  * \retval pointer on success (must be unreffed)
@@ -192,6 +194,8 @@ struct ast_channel_snapshot *ast_channel_snapshot_get_latest_by_name(const char
  * The given \a blob should be treated as immutable and not modified after it is
  * put into the message.
  *
+ * \pre chan is locked
+ *
  * \param chan Channel blob is associated with, or \c NULL for global/all channels.
  * \param type Message type for this blob.
  * \param blob JSON object representing the data, or \c NULL for no data. If
@@ -305,6 +309,8 @@ void ast_multi_channel_blob_add_channel(struct ast_multi_channel_blob *obj,
  * \brief Publish a channel blob message.
  * \since 12.0.0
  *
+ * \pre chan is locked
+ *
  * \param chan Channel publishing the blob.
  * \param type Type of stasis message.
  * \param blob The blob being published. (NULL if no blob)
@@ -318,6 +324,8 @@ void ast_channel_publish_blob(struct ast_channel *chan, struct stasis_message_ty
  * \since 12
  * \brief Set flag to indicate channel snapshot is being staged.
  *
+ * \pre chan is locked
+ *
  * \param chan Channel being staged.
  */
 void ast_channel_stage_snapshot(struct ast_channel *chan);
@@ -326,6 +334,8 @@ void ast_channel_stage_snapshot(struct ast_channel *chan);
  * \since 12
  * \brief Clear flag to indicate channel snapshot is being staged, and publish snapshot.
  *
+ * \pre chan is locked
+ *
  * \param chan Channel being staged.
  */
 void ast_channel_stage_snapshot_done(struct ast_channel *chan);
@@ -334,6 +344,8 @@ void ast_channel_stage_snapshot_done(struct ast_channel *chan);
  * \since 12
  * \brief Publish a \ref ast_channel_snapshot for a channel.
  *
+ * \pre chan is locked
+ *
  * \param chan Channel to publish.
  */
 void ast_channel_publish_snapshot(struct ast_channel *chan);
@@ -342,6 +354,8 @@ void ast_channel_publish_snapshot(struct ast_channel *chan);
  * \since 12
  * \brief Publish a \ref ast_channel_varset for a channel.
  *
+ * \pre chan is locked
+ *
  * \param chan Channel to publish the event for, or \c NULL for 'none'.
  * \param variable Name of the variable being set
  * \param value Value.
@@ -535,6 +549,8 @@ void ast_channel_publish_dial_forward(struct ast_channel *caller,
  * \brief Publish in the \ref ast_channel_topic a \ref ast_channel_snapshot
  * message indicating a change in channel state
  *
+ * \pre chan is locked
+ *
  * \param chan The channel whose state has changed
  */
 void ast_publish_channel_state(struct ast_channel *chan);