freetdm: - Update ftmod_sangoma_isdn to use core state advance

- Added locking documentation (docs/locking.txt)
	 - Updated core state advance to use state processor in span
           rather than pushing the processor in the function arguments

libs/freetdm/src/ftdm_state.c

@@ -434,15 +434,14 @@ FT_DECLARE(char *) ftdm_channel_get_history_str(const ftdm_channel_t *fchan)
 	return stream.data;
 }
 
-FT_DECLARE(ftdm_status_t) ftdm_channel_advance_states(ftdm_channel_t *fchan, ftdm_channel_state_processor_t state_processor)
+FT_DECLARE(ftdm_status_t) ftdm_channel_advance_states(ftdm_channel_t *fchan)
 {
 	ftdm_channel_state_t state;
 	
-	ftdm_channel_lock(fchan);
 	while (fchan->state_status == FTDM_STATE_STATUS_NEW) {
 		state = fchan->state;
 		ftdm_log_chan(fchan, FTDM_LOG_DEBUG, "Executing state processor for %s\n", ftdm_channel_state2str(fchan->state));
-		state_processor(fchan);
+		fchan->span->state_processor(fchan);
 		if (state == fchan->state && fchan->state_status == FTDM_STATE_STATUS_NEW) {
 			/* if the state did not change and is still NEW, the state status must go to PROCESSED
 			 * otherwise we don't touch it since is a new state and the old state was
@@ -451,7 +450,7 @@ FT_DECLARE(ftdm_status_t) ftdm_channel_advance_states(ftdm_channel_t *fchan, ftd
 			fchan->state_status = FTDM_STATE_STATUS_PROCESSED;
 		}
 	}
-	ftdm_channel_unlock(fchan);
+
 	return FTDM_SUCCESS;
 }
 

libs/freetdm/src/ftmod/ftmod_r2/ftmod_r2.c

@@ -620,7 +620,7 @@ static void ftdm_r2_on_call_init(openr2_chan_t *r2chan)
 			ftdm_sched_cancel_timer(r2data->sched, r2call->protocol_error_recovery_timer);
 			ftdm_log_chan_msg(ftdmchan, FTDM_LOG_DEBUG, "Cancelled protocol error recovery timer\n");
 			ftdm_set_state(ftdmchan, FTDM_CHANNEL_STATE_DOWN);
-			ftdm_channel_advance_states(ftdmchan, ftdm_r2_state_advance);
+			ftdm_channel_advance_states(ftdmchan);
 		}
 	}
 
@@ -814,7 +814,7 @@ static void ftdm_r2_on_call_end(openr2_chan_t *r2chan)
 	ftdm_set_state(ftdmchan, FTDM_CHANNEL_STATE_DOWN);
 
 	/* in some circumstances openr2 can call on_call_init right after this, so let's advance the state right here */
-	ftdm_channel_advance_states(ftdmchan, ftdm_r2_state_advance);
+	ftdm_channel_advance_states(ftdmchan);
 }
 
 static void ftdm_r2_on_call_read(openr2_chan_t *r2chan, const unsigned char *buf, int buflen)
@@ -846,7 +846,7 @@ static void ftdm_r2_recover_from_protocol_error(void *data)
 		goto done;
 	}
 	ftdm_set_state(ftdmchan, FTDM_CHANNEL_STATE_DOWN);
-	ftdm_channel_advance_states(ftdmchan, ftdm_r2_state_advance);
+	ftdm_channel_advance_states(ftdmchan);
 done:
 	ftdm_channel_unlock(ftdmchan);
 }
@@ -1584,6 +1584,7 @@ static FIO_CONFIGURE_SPAN_SIGNALING_FUNCTION(ftdm_r2_configure_span_signaling)
 	span->set_channel_sig_status = ftdm_r2_set_channel_sig_status;
 
 	span->state_map = &r2_state_map;
+	span->state_processor = ftdm_r2_state_advance;
 
 	/* use signals queue */
 	ftdm_set_flag(span, FTDM_SPAN_USE_SIGNALS_QUEUE);
@@ -1917,12 +1918,12 @@ static void *ftdm_r2_run(ftdm_thread_t *me, void *obj)
 			ftdm_clear_flag(ftdmchan, FTDM_CHANNEL_RX_DISABLED);
 			ftdm_clear_flag(ftdmchan, FTDM_CHANNEL_TX_DISABLED);
 
-			ftdm_channel_advance_states(ftdmchan, ftdm_r2_state_advance);
+			ftdm_channel_advance_states(ftdmchan);
 
 			r2chan = call->r2chan;
 			openr2_chan_process_signaling(r2chan);
 
-			ftdm_channel_advance_states(ftdmchan, ftdm_r2_state_advance);
+			ftdm_channel_advance_states(ftdmchan);
 
 			if (!call->accepted) {
 				/* if the call is not accepted we do not want users reading */

libs/freetdm/src/ftmod/ftmod_sangoma_isdn/ftmod_sangoma_isdn.c

@@ -46,10 +46,9 @@ static ftdm_status_t ftdm_sangoma_isdn_stop(ftdm_span_t *span);
 static ftdm_status_t ftdm_sangoma_isdn_start(ftdm_span_t *span);
 
 ftdm_channel_t* ftdm_sangoma_isdn_process_event_states(ftdm_span_t *span, sngisdn_event_data_t *sngisdn_event);
-static void ftdm_sangoma_isdn_advance_chan_states(ftdm_channel_t *ftdmchan);
 static void ftdm_sangoma_isdn_poll_events(ftdm_span_t *span);
 static void ftdm_sangoma_isdn_process_phy_events(ftdm_span_t *span, ftdm_oob_event_t event);
-static void ftdm_sangoma_isdn_process_state_change(ftdm_channel_t *ftdmchan);
+static ftdm_status_t ftdm_sangoma_isdn_process_state_change(ftdm_channel_t *ftdmchan);
 static void ftdm_sangoma_isdn_process_stack_event (ftdm_span_t *span, sngisdn_event_data_t *sngisdn_event);
 static void ftdm_sangoma_isdn_wakeup_phy(ftdm_channel_t *dchan);
 static void ftdm_sangoma_isdn_dchan_set_queue_size(ftdm_channel_t *ftdmchan);
@@ -270,13 +269,6 @@ ftdm_state_map_t sangoma_isdn_state_map = {
 	}
 };
 
-static __inline__ void ftdm_sangoma_isdn_advance_chan_states(ftdm_channel_t *ftdmchan)
-{
-	while (ftdm_test_flag(ftdmchan, FTDM_CHANNEL_STATE_CHANGE)) {
-		ftdm_sangoma_isdn_process_state_change(ftdmchan);
-	}
-}
-
 static void ftdm_sangoma_isdn_process_phy_events(ftdm_span_t *span, ftdm_oob_event_t event)
 {
 	sngisdn_span_data_t *signal_data = (sngisdn_span_data_t*) span->signal_data;
@@ -457,7 +449,7 @@ static void *ftdm_sangoma_isdn_run(ftdm_thread_t *me, void *obj)
 				while ((ftdmchan = ftdm_queue_dequeue(span->pendingchans))) {
 					/* double check that this channel has a state change pending */
 					ftdm_channel_lock(ftdmchan);
-					ftdm_sangoma_isdn_advance_chan_states(ftdmchan);
+					ftdm_channel_advance_states(ftdmchan);
 					ftdm_channel_unlock(ftdmchan);
 				}
 
@@ -470,11 +462,11 @@ static void *ftdm_sangoma_isdn_run(ftdm_thread_t *me, void *obj)
 				/* twiddle */
 				break;
 			case FTDM_FAIL:
-				ftdm_log(FTDM_LOG_ERROR,"%s:ftdm_interrupt_wait returned error!\n", span->name);
+				ftdm_log(FTDM_LOG_ERROR, "%s: ftdm_interrupt_wait returned error!\n", span->name);
 				break;
 
 			default:
-				ftdm_log(FTDM_LOG_ERROR,"%s:ftdm_interrupt_wait returned with unknown code\n", span->name);
+				ftdm_log(FTDM_LOG_ERROR, "%s: ftdm_interrupt_wait returned with unknown code\n", span->name);
 				break;
 		}
 
@@ -536,7 +528,7 @@ ftdm_channel_t* ftdm_sangoma_isdn_process_event_states(ftdm_span_t *span, sngisd
 			break;
 	}
  	ftdm_channel_lock(ftdmchan);
-	ftdm_sangoma_isdn_advance_chan_states(ftdmchan);
+	ftdm_channel_advance_states(ftdmchan);
 	return ftdmchan;
 }
 
@@ -600,13 +592,14 @@ static void ftdm_sangoma_isdn_process_stack_event (ftdm_span_t *span, sngisdn_ev
 			sngisdn_process_rst_ind(sngisdn_event);
 			break;
 	}
-	if(ftdmchan != NULL) {
-		ftdm_sangoma_isdn_advance_chan_states(ftdmchan);
+	if (ftdmchan != NULL) {
+		ftdm_channel_advance_states(ftdmchan);
 		ftdm_channel_unlock(ftdmchan);
 	}
 }
 
-static void ftdm_sangoma_isdn_process_state_change(ftdm_channel_t *ftdmchan)
+/* this function is called with the channel already locked by the core */
+static ftdm_status_t ftdm_sangoma_isdn_process_state_change(ftdm_channel_t *ftdmchan)
 {
 	ftdm_sigmsg_t		sigev;
 	ftdm_channel_state_t initial_state;
@@ -618,13 +611,12 @@ static void ftdm_sangoma_isdn_process_state_change(ftdm_channel_t *ftdmchan)
 	sigev.span_id = ftdmchan->span_id;
 	sigev.channel = ftdmchan;
 
-	/*first lock the channel*/
-	ftdm_channel_lock(ftdmchan);
-	/*clear the state change flag...since we might be setting a new state*/
-	ftdm_clear_flag(ftdmchan, FTDM_CHANNEL_STATE_CHANGE);
+	/* Acknowledge the state change */
+	ftdm_channel_complete_state(ftdmchan);
+
 #ifdef FTDM_DEBUG_CHAN_MEMORY
 	if (ftdmchan->state == FTDM_CHANNEL_STATE_DIALING) {
-		ftdm_assert(mprotect(ftdmchan, sizeof(*ftdmchan), PROT_READ)==0, "Failed to mprotect");
+		ftdm_assert(mprotect(ftdmchan, sizeof(*ftdmchan), PROT_READ) == 0, "Failed to mprotect");
 	}
 #endif
 	
@@ -879,11 +871,10 @@ static void ftdm_sangoma_isdn_process_state_change(ftdm_channel_t *ftdmchan)
 	}
 #ifdef FTDM_DEBUG_CHAN_MEMORY
 	if (ftdmchan->state == FTDM_CHANNEL_STATE_DIALING) {
-		ftdm_assert(mprotect(ftdmchan, sizeof(*ftdmchan), PROT_READ|PROT_WRITE)==0, "Failed to mprotect");
+		ftdm_assert(mprotect(ftdmchan, sizeof(*ftdmchan), PROT_READ|PROT_WRITE) == 0, "Failed to mprotect");
 	}
 #endif
-	ftdm_channel_unlock(ftdmchan);
-	return;
+	return FTDM_SUCCESS;
 }
 
 static FIO_CHANNEL_SEND_MSG_FUNCTION(ftdm_sangoma_isdn_send_msg)
@@ -1098,6 +1089,7 @@ static FIO_CONFIGURE_SPAN_SIGNALING_FUNCTION(ftdm_sangoma_isdn_span_config)
 	span->get_span_sig_status = ftdm_sangoma_isdn_get_span_sig_status;
 	span->set_span_sig_status = ftdm_sangoma_isdn_set_span_sig_status;
 	span->state_map	= &sangoma_isdn_state_map;
+	span->state_processor = ftdm_sangoma_isdn_process_state_change;
 	ftdm_set_flag(span, FTDM_SPAN_USE_CHAN_QUEUE);
 	ftdm_set_flag(span, FTDM_SPAN_USE_SIGNALS_QUEUE);
 	ftdm_set_flag(span, FTDM_SPAN_USE_PROCEED_STATE);

libs/freetdm/src/include/freetdm.h

@@ -356,7 +356,6 @@ typedef enum {
 		"PROGRESS_MEDIA", "ALARM_TRAP", "ALARM_CLEAR", \
 		"COLLECTED_DIGIT", "ADD_CALL", "RESTART", "SIGSTATUS_CHANGED", "COLLISION", "FACILITY", \
 		"TRACE", "TRACE_RAW", "INDICATION_COMPLETED", "INVALID"
-
 /*! \brief Move from string to ftdm_signal_event_t and viceversa */
 FTDM_STR2ENUM_P(ftdm_str2ftdm_signal_event, ftdm_signal_event2str, ftdm_signal_event_t)
 
@@ -653,7 +652,20 @@ typedef ftdm_status_t (*fio_span_get_sig_status_t) FIO_SPAN_GET_SIG_STATUS_ARGS;
 typedef ftdm_status_t (*fio_span_poll_event_t) FIO_SPAN_POLL_EVENT_ARGS ;
 typedef ftdm_status_t (*fio_span_next_event_t) FIO_SPAN_NEXT_EVENT_ARGS ;
 typedef ftdm_status_t (*fio_channel_next_event_t) FIO_CHANNEL_NEXT_EVENT_ARGS ;
+
+/*! \brief Callback for signal delivery (FTDM_SIGEVENT_START and friends) 
+ *  \note This callback is provided by the user during ftdm_configure_span_signaling
+ *
+ *  \note You must NOT do any blocking during this callback since this function is
+ *        most likely called in an internal signaling thread that can potentially be
+ *        shared for all the channels in a span and blocking will delay processing
+ *        (sometimes even audio processing) for other channels
+ *
+ *  \note Although some simple FreeTDM APIs can work (ie: ftdm_span_get_id etc), the
+ *        use of any FreeTDM call API (ie ftdm_channel_call_answer) is discouraged
+ */
 typedef ftdm_status_t (*fio_signal_cb_t) FIO_SIGNAL_CB_ARGS ;
+
 typedef ftdm_status_t (*fio_event_cb_t) FIO_EVENT_CB_ARGS ;
 typedef ftdm_status_t (*fio_configure_span_t) FIO_CONFIGURE_SPAN_ARGS ;
 typedef ftdm_status_t (*fio_configure_t) FIO_CONFIGURE_ARGS ;

libs/freetdm/src/include/private/ftdm_core.h

@@ -500,15 +500,15 @@ struct ftdm_span {
 	ftdm_span_stop_t stop;
 	ftdm_channel_sig_read_t sig_read;
 	ftdm_channel_sig_write_t sig_write;
-	/* Private I/O data per span. Do not touch unless you are an I/O module */
-	void *io_data;
+	ftdm_channel_state_processor_t state_processor; /*!< This guy is called whenever state processing is required */
+	void *io_data; /*!< Private I/O data per span. Do not touch unless you are an I/O module */
 	char *type;
 	char *dtmf_hangup;
 	size_t dtmf_hangup_len;
 	ftdm_state_map_t *state_map;
 	ftdm_caller_data_t default_caller_data;
-	ftdm_queue_t *pendingchans;
-	ftdm_queue_t *pendingsignals;
+	ftdm_queue_t *pendingchans; /*!< Channels pending of state processing */
+	ftdm_queue_t *pendingsignals; /*!< Signals pending from being delivered to the user */
 	struct ftdm_span *next;
 };
 

libs/freetdm/src/include/private/ftdm_state.h

@@ -86,15 +86,21 @@ typedef struct {
 	const char *file;
 	const char *func;
 	int line;
-	ftdm_channel_state_t state;
-	ftdm_channel_state_t last_state;
-	ftdm_time_t time;
-	ftdm_time_t end_time;
+	ftdm_channel_state_t state; /*!< Current state (processed or not) */
+	ftdm_channel_state_t last_state; /*!< Previous state */
+	ftdm_time_t time; /*!< Time the state was set */
+	ftdm_time_t end_time; /*!< Time the state processing was completed */
 } ftdm_state_history_entry_t;
 
 typedef ftdm_status_t (*ftdm_channel_state_processor_t)(ftdm_channel_t *fchan);
 
-FT_DECLARE(ftdm_status_t) ftdm_channel_advance_states(ftdm_channel_t *fchan, ftdm_channel_state_processor_t processor);
+/*!
+ * \brief Process channel states by invoking the channel state processing routine
+ *        it will keep calling the processing routine while the state status
+ *        is FTDM_STATE_STATUS_NEW, it will not do anything otherwise
+ */
+FT_DECLARE(ftdm_status_t) ftdm_channel_advance_states(ftdm_channel_t *fchan);
+
 FT_DECLARE(ftdm_status_t) _ftdm_channel_complete_state(const char *file, const char *function, int line, ftdm_channel_t *fchan);
 #define ftdm_channel_complete_state(obj) _ftdm_channel_complete_state(__FILE__, __FUNCTION__, __LINE__, obj)
 FT_DECLARE(int) ftdm_check_state_all(ftdm_span_t *span, ftdm_channel_state_t state);
@@ -171,16 +177,26 @@ struct ftdm_state_map {
 };
 typedef struct ftdm_state_map ftdm_state_map_t;
 
+/*!\brief Set the state for a channel (the channel must be locked when calling this function)
+ * \note Signaling modules should use ftdm_set_state macro instead
+ * \note If this function is called with the wait parameter set to a non-zero value, the recursivity
+ *       of the channel lock must be == 1 because the channel will be unlocked/locked when waiting */
 FT_DECLARE(ftdm_status_t) ftdm_channel_set_state(const char *file, const char *func, int line,
 		ftdm_channel_t *ftdmchan, ftdm_channel_state_t state, int wait);
 
-/*!\brief Set the state of a channel immediately and implicitly complete the previous state */
+/*!\brief Set the state of a channel immediately and implicitly complete the previous state if needed 
+ * \note FTDM_SIGEVENT_INDICATION_COMPLETED will be sent if the state change 
+ *       is associated to some indication (ie FTDM_CHANNEL_INDICATE_PROCEED)
+ * \note The channel must be locked when calling this function
+ * */
 FT_DECLARE(ftdm_status_t) _ftdm_set_state(const char *file, const char *func, int line,
 			ftdm_channel_t *fchan, ftdm_channel_state_t state);
 #define ftdm_set_state(obj, s) _ftdm_set_state(__FILE__, __FUNCTION__, __LINE__, obj, s);									\
 
 /*!\brief This macro is deprecated, signaling modules should always lock the channel themselves anyways since they must
  * process first the user pending state changes then set a new state before releasing the lock 
+ * this macro is here for backwards compatibility, DO NOT USE IT in new code since it is *always* wrong to set
+ * a state in a signaling module without checking and processing the current state first (and for that you must lock the channel)
  */
 #define ftdm_set_state_locked(obj, s) \
 	do { \