Ticket #411: Cannot update account presence's status while previous PUBLISH is in progress (thanks Olivier Beytrison for the report)

 - enable request queueing. If PUBLISH is to be sent while another one is still in progress, queue the request and send it later when the ongoing request completes
 - this behavior is controlled by new pjsip_publishc_opt structure to control session's options
 - default behavior is to queue the request


git-svn-id: http://svn.pjsip.org/repos/pjproject/trunk@2940 74dad513-b988-da41-8d7b-12977e46ad98

6 files changed, 149 insertions, 15 deletions

diff --git a/pjsip/include/pjsip-simple/publish.h b/pjsip/include/pjsip-simple/publish.h
index 3c7c7d55..a05eb6bc 100644
--- a/pjsip/include/pjsip-simple/publish.h
+++ b/pjsip/include/pjsip-simple/publish.h
@@ -65,6 +65,25 @@ extern const pjsip_method pjsip_publish_method;
 typedef struct pjsip_publishc pjsip_publishc;
 
 
+/**
+ * Client publication options. Application should initialize this structure
+ * with its default values by calling #pjsip_publishc_opt_default()
+ */
+typedef struct pjsip_publishc_opt
+{
+    /**
+     * Specify whether the client publication session should queue the
+     * PUBLISH request should there be another PUBLISH transaction still
+     * pending. If this is set to false, the client will return error
+     * on the PUBLISH request if there is another PUBLISH transaction still
+     * in progress.
+     *
+     * Default: PJSIP_PUBLISHC_QUEUE_REQUEST
+     */
+    pj_bool_t	queue_request;
+
+} pjsip_publishc_opt;
+
 
 /** Structure to hold parameters when calling application's callback.
  *  The application's callback is called when the client publication process
@@ -89,6 +108,14 @@ typedef void pjsip_publishc_cb(struct pjsip_publishc_cbparam *param);
 
 
 /**
+ * Initialize client publication session option with default values.
+ *
+ * @param opt	    The option.
+ */
+PJ_DECL(void) pjsip_publishc_opt_default(pjsip_publishc_opt *opt);
+
+
+/**
  * Initialize client publication module.
  *
  * @param endpt	    SIP endpoint.
@@ -98,12 +125,11 @@ typedef void pjsip_publishc_cb(struct pjsip_publishc_cbparam *param);
 PJ_DECL(pj_status_t) pjsip_publishc_init_module(pjsip_endpoint *endpt);
 
 
-
 /**
  * Create client publication structure.
  *
  * @param endpt	    Endpoint, used to allocate pool from.
- * @param options   Option flags.
+ * @param opt	    Options, or NULL to specify default options.
  * @param token	    Opaque data to be associated with the client publication.
  * @param cb	    Pointer to callback function to receive publication status.
  * @param p_pubc    Pointer to receive client publication structure.
@@ -111,7 +137,7 @@ PJ_DECL(pj_status_t) pjsip_publishc_init_module(pjsip_endpoint *endpt);
  * @return	    PJ_SUCCESS on success.
  */
 PJ_DECL(pj_status_t) pjsip_publishc_create( pjsip_endpoint *endpt, 
-					    unsigned options,
+					    const pjsip_publishc_opt *opt,
 					    void *token,
 				            pjsip_publishc_cb *cb, 
 					    pjsip_publishc **p_pubc);
@@ -269,10 +295,23 @@ PJ_DECL(pj_status_t) pjsip_publishc_update_expires(pjsip_publishc *pubc,
  * and application will be notified via the callback when the process 
  * completes.
  *
+ * If the session has another PUBLISH request outstanding, the behavior
+ * depends on whether request queueing is enabled in the session (this was
+ * set by setting \a queue_request field of #pjsip_publishc_opt to true
+ * when calling #pjsip_publishc_create(). Default is true). If request
+ * queueing is enabled, the request will be queued and the function will 
+ * return PJ_EPENDING. One the outstanding request is complete, the queued
+ * request will be sent automatically. If request queueing is disabled, the
+ * function will reject the request and return PJ_EBUSY.
+ *
  * @param pubc	    The client publication structure.
  * @param tdata	    Transmit data.
  *
- * @return	    PJ_SUCCESS on success.
+ * @return	    - PJ_SUCCESS on success, or 
+ *		    - PJ_EPENDING if request is queued, or
+ *		    - PJ_EBUSY if request is rejected because another PUBLISH
+ *		      request is in progress, or
+ *		    - other status code to indicate the error.
  */
 PJ_DECL(pj_status_t) pjsip_publishc_send(pjsip_publishc *pubc, 
 					 pjsip_tx_data *tdata);
diff --git a/pjsip/include/pjsip/sip_config.h b/pjsip/include/pjsip/sip_config.h
index af38c040..5bbe420f 100644
--- a/pjsip/include/pjsip/sip_config.h
+++ b/pjsip/include/pjsip/sip_config.h
@@ -896,6 +896,20 @@ PJ_INLINE(pjsip_cfg_t*) pjsip_cfg(void)
 #endif
 
 
+/**
+ * Specify whether the client publication session should queue the
+ * PUBLISH request should there be another PUBLISH transaction still
+ * pending. If this is set to false, the client will return error
+ * on the PUBLISH request if there is another PUBLISH transaction still
+ * in progress.
+ *
+ * Default: 1 (yes)
+ */
+#ifndef PJSIP_PUBLISHC_QUEUE_REQUEST
+#   define PJSIP_PUBLISHC_QUEUE_REQUEST		1
+#endif
+
+
 PJ_END_DECL
 
 /**
diff --git a/pjsip/include/pjsua-lib/pjsua.h b/pjsip/include/pjsua-lib/pjsua.h
index cd86a524..bb6558db 100644
--- a/pjsip/include/pjsua-lib/pjsua.h
+++ b/pjsip/include/pjsua-lib/pjsua.h
@@ -1797,6 +1797,11 @@ typedef struct pjsua_acc_config
     pj_bool_t	    publish_enabled;
 
     /**
+     * Event publication options.
+     */
+    pjsip_publishc_opt	publish_opt;
+
+    /**
      * Authentication preference.
      */
     pjsip_auth_clt_pref auth_pref;
diff --git a/pjsip/src/pjsip-simple/publishc.c b/pjsip/src/pjsip-simple/publishc.c
index 6ba867a8..6313ba96 100644
--- a/pjsip/src/pjsip-simple/publishc.c
+++ b/pjsip/src/pjsip-simple/publishc.c
@@ -57,6 +57,15 @@ const pjsip_method pjsip_publish_method =
 
 
 /**
+ * Pending request list.
+ */
+typedef struct pending_publish
+{
+    PJ_DECL_LIST_MEMBER(pjsip_tx_data);
+} pending_publish;
+
+
+/**
  * SIP client publication structure.
  */
 struct pjsip_publishc
@@ -65,7 +74,9 @@ struct pjsip_publishc
     pjsip_endpoint		*endpt;
     pj_bool_t			 _delete_flag;
     int				 pending_tsx;
+    pj_mutex_t			*mutex;
 
+    pjsip_publishc_opt		 opt;
     void			*token;
     pjsip_publishc_cb		*cb;
 
@@ -91,9 +102,18 @@ struct pjsip_publishc
     pj_time_val			 last_refresh;
     pj_time_val			 next_refresh;
     pj_timer_entry		 timer;
+
+    /* Pending PUBLISH request */
+    pending_publish		 pending_reqs;
 };
 
 
+PJ_DEF(void) pjsip_publishc_opt_default(pjsip_publishc_opt *opt)
+{
+    pj_bzero(opt, sizeof(*opt));
+    opt->queue_request = PJSIP_PUBLISHC_QUEUE_REQUEST;
+}
+
 
 /*
  * Initialize client publication module.
@@ -127,20 +147,18 @@ PJ_DEF(pj_status_t) pjsip_publishc_init_module(pjsip_endpoint *endpt)
 
 
 PJ_DEF(pj_status_t) pjsip_publishc_create( pjsip_endpoint *endpt, 
-					   unsigned options,
+					   const pjsip_publishc_opt *opt,
 					   void *token,
 					   pjsip_publishc_cb *cb,	
 					   pjsip_publishc **p_pubc)
 {
     pj_pool_t *pool;
     pjsip_publishc *pubc;
+    pjsip_publishc_opt default_opt;
     pj_status_t status;
 
     /* Verify arguments. */
     PJ_ASSERT_RETURN(endpt && cb && p_pubc, PJ_EINVAL);
-    PJ_ASSERT_RETURN(options == 0, PJ_EINVAL);
-
-    PJ_UNUSED_ARG(options);
 
     pool = pjsip_endpt_create_pool(endpt, "pubc%p", 1024, 1024);
     PJ_ASSERT_RETURN(pool != NULL, PJ_ENOMEM);
@@ -153,9 +171,25 @@ PJ_DEF(pj_status_t) pjsip_publishc_create( pjsip_endpoint *endpt,
     pubc->cb = cb;
     pubc->expires = PJSIP_PUBC_EXPIRATION_NOT_SPECIFIED;
 
+    if (!opt) {
+	pjsip_publishc_opt_default(&default_opt);
+	opt = &default_opt;
+    }
+    pj_memcpy(&pubc->opt, opt, sizeof(*opt));
+    pj_list_init(&pubc->pending_reqs);
+
+    status = pj_mutex_create_recursive(pubc->pool, "pubc%p", &pubc->mutex);
+    if (status != PJ_SUCCESS) {
+	pj_pool_release(pool);
+	return status;
+    }
+
     status = pjsip_auth_clt_init(&pubc->auth_sess, endpt, pubc->pool, 0);
-    if (status != PJ_SUCCESS)
+    if (status != PJ_SUCCESS) {
+	pj_mutex_destroy(pubc->mutex);
+	pj_pool_release(pool);
 	return status;
+    }
 
     pj_list_init(&pubc->route_set);
     pj_list_init(&pubc->usr_hdr);
@@ -180,6 +214,8 @@ PJ_DEF(pj_status_t) pjsip_publishc_destroy(pjsip_publishc *pubc)
 	    pubc->timer.id = 0;
 	}
 
+	if (pubc->mutex)
+	    pj_mutex_destroy(pubc->mutex);
 	pjsip_endpt_release_pool(pubc->endpt, pubc->pool);
     }
 
@@ -576,6 +612,12 @@ static void tsx_callback(void *token, pjsip_event *event)
 	    if (pubc->auto_refresh && expiration!=0 && expiration!=0xFFFF) {
 		pj_time_val delay = { 0, 0};
 
+		/* Cancel existing timer, if any */
+		if (pubc->timer.id != 0) {
+		    pjsip_endpt_cancel_timer(pubc->endpt, &pubc->timer);
+		    pubc->timer.id = 0;
+		}
+
 		delay.sec = expiration - DELAY_BEFORE_REFRESH;
 		if (pubc->expires != PJSIP_PUBC_EXPIRATION_NOT_SPECIFIED && 
 		    delay.sec > (pj_int32_t)pubc->expires) 
@@ -613,6 +655,22 @@ static void tsx_callback(void *token, pjsip_event *event)
 		      rdata, expiration);
 
 	--pubc->pending_tsx;
+
+	/* If we have pending request(s), send them now */
+	pj_mutex_lock(pubc->mutex);
+	while (!pj_list_empty(&pubc->pending_reqs)) {
+	    pjsip_tx_data *tdata = pubc->pending_reqs.next;
+	    pj_list_erase(tdata);
+	    status = pjsip_publishc_send(pubc, tdata);
+	    if (status == PJ_EPENDING) {
+		pj_assert(!"Not expected");
+		pj_list_erase(tdata);
+		pjsip_tx_data_dec_ref(tdata);
+	    } else if (status == PJ_SUCCESS) {
+		break;
+	    }
+	}
+	pj_mutex_unlock(pubc->mutex);
     }
 
     /* Delete the record if user destroy pubc during the callback. */
@@ -629,13 +687,26 @@ PJ_DEF(pj_status_t) pjsip_publishc_send(pjsip_publishc *pubc,
     pjsip_cseq_hdr *cseq_hdr;
     pj_uint32_t cseq;
 
+    PJ_ASSERT_RETURN(pubc && tdata, PJ_EINVAL);
+
     /* Make sure we don't have pending transaction. */
+    pj_mutex_lock(pubc->mutex);
     if (pubc->pending_tsx) {
-	PJ_LOG(4,(THIS_FILE, "Unable to send request, pubc has another "
-			     "transaction pending"));
-	pjsip_tx_data_dec_ref( tdata );
-	return PJSIP_EBUSY;
+	if (pubc->opt.queue_request) {
+	    pj_list_push_back(&pubc->pending_reqs, tdata);
+	    pj_mutex_unlock(pubc->mutex);
+	    PJ_LOG(4,(THIS_FILE, "Request is queued, pubc has another "
+				 "transaction pending"));
+	    return PJ_EPENDING;
+	} else {
+	    pjsip_tx_data_dec_ref(tdata);
+	    pj_mutex_unlock(pubc->mutex);
+	    PJ_LOG(4,(THIS_FILE, "Unable to send request, pubc has another "
+				 "transaction pending"));
+	    return PJ_EBUSY;
+	}
     }
+    pj_mutex_unlock(pubc->mutex);
 
     /* Invalidate message buffer. */
     pjsip_tx_data_invalidate_msg(tdata);
diff --git a/pjsip/src/pjsua-lib/pjsua_core.c b/pjsip/src/pjsua-lib/pjsua_core.c
index 0b24cb5c..cc26bc87 100644
--- a/pjsip/src/pjsua-lib/pjsua_core.c
+++ b/pjsip/src/pjsua-lib/pjsua_core.c
@@ -161,6 +161,7 @@ PJ_DEF(void) pjsua_acc_config_default(pjsua_acc_config *cfg)
     pj_bzero(cfg, sizeof(*cfg));
 
     cfg->reg_timeout = PJSUA_REG_INTERVAL;
+    pjsip_publishc_opt_default(&cfg->publish_opt);
     cfg->transport_id = PJSUA_INVALID_ID;
     cfg->allow_contact_rewrite = PJ_TRUE;
     cfg->require_100rel = pjsua_var.ua_cfg.require_100rel;
diff --git a/pjsip/src/pjsua-lib/pjsua_pres.c b/pjsip/src/pjsua-lib/pjsua_pres.c
index b9c11ae4..bef8af92 100644
--- a/pjsip/src/pjsua-lib/pjsua_pres.c
+++ b/pjsip/src/pjsua-lib/pjsua_pres.c
@@ -1073,7 +1073,10 @@ static pj_status_t send_publish(int acc_id, pj_bool_t active)
 
     /* Send the PUBLISH request */
     status = pjsip_publishc_send(acc->publish_sess, tdata);
-    if (status != PJ_SUCCESS) {
+    if (status == PJ_EPENDING) {
+	PJ_LOG(3,(THIS_FILE, "Previous request is in progress, "
+		  "PUBLISH request is queued"));
+    } else if (status != PJ_SUCCESS) {
 	pjsua_perror(THIS_FILE, "Error sending PUBLISH request", status);
 	goto on_error;
     }
@@ -1102,7 +1105,8 @@ pj_status_t pjsua_pres_init_publish_acc(int acc_id)
     if (acc_cfg->publish_enabled) {
 
 	/* Create client publication */
-	status = pjsip_publishc_create(pjsua_var.endpt, 0, acc, &publish_cb,
+	status = pjsip_publishc_create(pjsua_var.endpt, &acc_cfg->publish_opt, 
+				       acc, &publish_cb,
 				       &acc->publish_sess);
 	if (status != PJ_SUCCESS) {
 	    acc->publish_sess = NULL;