Merged revisions 108135 via svnmerge from

https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r108135 | russell | 2008-03-12 14:57:42 -0500 (Wed, 12 Mar 2008) | 40 lines

(closes issue #12187, reported by atis, fixed by me after some brainstorming
 on the issue with mmichelson)

- Update copyright info on app_chanspy.

- Fix a race condition that caused app_chanspy to crash.  The issue was that
  the chanspy datastore magic that was used to ensure that spyee channels did
  not disappear out from under the code did not completely solve the problem.
  It was actually possible for chanspy to acquire a channel reference out of
  its datastore to a channel that was in the middle of being destroyed.  That
  was because datastore destruction in ast_channel_free() was done near the
  end.  So, this left the code in app_chanspy accessing a channel that was
  partially, or completely invalid because it was in the process of being free'd
  by another thread.  The following sort of shows the code path where the race 
  occurred:

  =============================================================================
  Thread 1 (PBX thread for spyee chan)  ||   Thread 2 (chanspy)
  --------------------------------------||-------------------------------------
  ast_channel_free()                    ||
    - remove channel from channel list  ||
    - lock/unlock the channel to ensure ||
      that no references retrieved from ||
      the channel list exist.           ||
  --------------------------------------||-------------------------------------
                                        || channel_spy()
    - destroy some channel data         ||  - Lock chanspy datastore
                                        ||  - Retrieve reference to channel
                                        ||  - lock channel
                                        ||  - Unlock chanspy datastore
  --------------------------------------||-------------------------------------
     - destroy channel datastores       ||
        - call chanspy datastore d'tor  ||  
          which NULL's out the ds'      ||  - Operate on the channel ...
          reference to the channel      ||     
                                        ||
    - free the channel                  || 
                                        ||
                                        ||  - unlock the channel
  --------------------------------------||-------------------------------------
  =============================================================================

........


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

2 files changed, 16 insertions, 9 deletions

diff --git a/apps/app_chanspy.c b/apps/app_chanspy.c
index 0bf676891..d7047d047 100644
--- a/apps/app_chanspy.c
+++ b/apps/app_chanspy.c
@@ -2,7 +2,7 @@
  * Asterisk -- An open source telephony toolkit.
  *
  * Copyright (C) 2005 Anthony Minessale II (anthmct@yahoo.com)
- * Copyright (C) 2005 - 2006, Digium, Inc.
+ * Copyright (C) 2005 - 2008, Digium, Inc.
  *
  * A license has been granted to Digium (via disclaimer) for the use of
  * this code.
@@ -23,6 +23,8 @@
  * \brief ChanSpy: Listen in on any channel.
  *
  * \author Anthony Minessale II <anthmct@yahoo.com>
+ * \author Joshua Colp <jcolp@digium.com>
+ * \author Russell Bryant <russell@digium.com>
  *
  * \ingroup applications
  */
diff --git a/main/channel.c b/main/channel.c
index 19d7a7c2c..93f6f58f3 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -1257,10 +1257,21 @@ void ast_channel_free(struct ast_channel *chan)
 		AST_RWLIST_UNLOCK(&channels);
 		ast_log(LOG_ERROR, "Unable to find channel in list to free. Assuming it has already been done.\n");
 	}
-	/* Lock and unlock the channel just to be sure nobody
-	   has it locked still */
+	/* Lock and unlock the channel just to be sure nobody has it locked still
+	   due to a reference retrieved from the channel list. */
 	ast_channel_lock(chan);
 	ast_channel_unlock(chan);
+
+	/* Get rid of each of the data stores on the channel */
+	while ((datastore = AST_LIST_REMOVE_HEAD(&chan->datastores, entry)))
+		/* Free the data store */
+		ast_channel_datastore_free(datastore);
+
+	/* Lock and unlock the channel just to be sure nobody has it locked still
+	   due to a reference that was stored in a datastore. (i.e. app_chanspy) */
+	ast_channel_lock(chan);
+	ast_channel_unlock(chan);
+
 	if (chan->tech_pvt) {
 		ast_log(LOG_WARNING, "Channel '%s' may not have been hung up properly\n", chan->name);
 		ast_free(chan->tech_pvt);
@@ -1304,12 +1315,6 @@ void ast_channel_free(struct ast_channel *chan)
 	while ((f = AST_LIST_REMOVE_HEAD(&chan->readq, frame_list)))
 		ast_frfree(f);
 	
-	/* Get rid of each of the data stores on the channel */
-	while ((datastore = AST_LIST_REMOVE_HEAD(&chan->datastores, entry)))
-		/* Free the data store */
-		ast_channel_datastore_free(datastore);
-	AST_LIST_HEAD_INIT_NOLOCK(&chan->datastores);
-
 	/* loop over the variables list, freeing all data and deleting list items */
 	/* no need to lock the list, as the channel is already locked */