Reinstate msc subscriber conn ref counting

Indeed the easiest way of ensuring all code paths can cause conn discarding
while still all code paths can check on whether a conn needs to be discarded.

Change-Id: I641fe36d9fa2077e3caf63cc583aaa380603bff0

7 files changed, 47 insertions, 52 deletions

diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c
index ade072097..01cae149e 100644
--- a/openbsc/src/libmsc/gsm_04_08.c
+++ b/openbsc/src/libmsc/gsm_04_08.c
@@ -199,7 +199,6 @@ void gsm0408_clear_request(struct gsm_subscriber_connection *conn, uint32_t caus
 		LOGP(DMM, LOGL_ERROR,
 		     "%s: Conn clear request on uninitialized conn\n",
 		     vlr_subscr_name(conn->vsub));
-		msc_subscr_con_free(conn);
 		return;
 	}
 
@@ -3362,7 +3361,7 @@ int mncc_tx_to_cc(struct gsm_network *net, int msg_type, void *arg)
 		}
 
 		/* Assign conn */
-		trans->conn = subscr_con_get(conn);
+		trans->conn = msc_conn_get(conn);
 		vlr_subscr_put(vsub);
 	} else {
 		/* update the subscriber we deal with */
@@ -3512,7 +3511,7 @@ static int gsm0408_rcv_cc(struct gsm_subscriber_connection *conn, struct msgb *m
 			return -ENOMEM;
 		}
 		/* Assign transaction */
-		trans->conn = subscr_con_get(conn);
+		trans->conn = msc_conn_get(conn);
 	}
 
 	/* find function for current state and message */
diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index d2a9723bf..cb02be2f7 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -832,7 +832,7 @@ int gsm0411_rcv_sms(struct gsm_subscriber_connection *conn,
 		gsm411_smr_init(&trans->sms.smr_inst, 0, 1,
 			gsm411_rl_recv, gsm411_mn_send);
 
-		trans->conn = subscr_con_get(conn);
+		trans->conn = msc_conn_get(conn);
 
 		new_trans = 1;
 	}
@@ -913,7 +913,7 @@ int gsm411_send_sms(struct gsm_subscriber_connection *conn, struct gsm_sms *sms)
 		gsm411_rl_recv, gsm411_mn_send);
 	trans->sms.sms = sms;
 
-	trans->conn = subscr_con_get(conn);
+	trans->conn = msc_conn_get(conn);
 
 	/* Hardcode SMSC Originating Address for now */
 	data = (uint8_t *)msgb_put(msg, 8);
diff --git a/openbsc/src/libmsc/iucs.c b/openbsc/src/libmsc/iucs.c
index 8d763d223..0ef09a820 100644
--- a/openbsc/src/libmsc/iucs.c
+++ b/openbsc/src/libmsc/iucs.c
@@ -169,7 +169,8 @@ int gsm0408_rcvmsg_iucs(struct gsm_network *network, struct msgb *msg,
 		uint8_t pdisc = gh->proto_discr & 0x0f;
 		OSMO_ASSERT(pdisc != GSM48_PDISC_RR);
 
-		rc = gsm0408_dispatch(conn, msg);
+		msc_dtap(conn, ue_ctx->conn_id, msg);
+		rc = 0;
 	} else {
 		/* allocate a new connection */
 
diff --git a/openbsc/src/libmsc/osmo_msc.c b/openbsc/src/libmsc/osmo_msc.c
index e7976f5ad..644c55c1c 100644
--- a/openbsc/src/libmsc/osmo_msc.c
+++ b/openbsc/src/libmsc/osmo_msc.c
@@ -48,38 +48,45 @@ static int msc_clear_request(struct gsm_subscriber_connection *conn, uint32_t ca
 	return 1;
 }
 
+static void subscr_conn_bump(struct gsm_subscriber_connection *conn)
+{
+	if (!conn)
+		return;
+	if (!conn->conn_fsm)
+		return;
+	if (!(conn->conn_fsm->state == SUBSCR_CONN_S_ACCEPTED
+	      || conn->conn_fsm->state == SUBSCR_CONN_S_COMMUNICATING))
+		return;
+	osmo_fsm_inst_dispatch(conn->conn_fsm, SUBSCR_CONN_E_BUMP, NULL);
+}
+
 /* receive a Level 3 Complete message and return MSC_CONN_ACCEPT or
  * MSC_CONN_REJECT */
 int msc_compl_l3(struct gsm_subscriber_connection *conn,
 		 struct msgb *msg, uint16_t chosen_channel)
 {
+	msc_conn_get(conn);
 	gsm0408_dispatch(conn, msg);
 
+	/* Bump whether the conn wants to be closed */
+	subscr_conn_bump(conn);
+	msc_conn_put(conn);
+
 	/* Always return acceptance, because even if the conn was not accepted,
 	 * we assumed ownership of it and the caller shall not interfere with
 	 * that. We may even already have discarded the conn. */
 	return MSC_CONN_ACCEPT;
 }
 
-static void subscr_conn_bump(struct gsm_subscriber_connection *conn)
-{
-	if (!conn)
-		return;
-	if (!conn->conn_fsm)
-		return;
-	if (!(conn->conn_fsm->state == SUBSCR_CONN_S_ACCEPTED
-	      || conn->conn_fsm->state == SUBSCR_CONN_S_COMMUNICATING))
-		return;
-	osmo_fsm_inst_dispatch(conn->conn_fsm, SUBSCR_CONN_E_BUMP, NULL);
-}
-
 /* Receive a DTAP message from BSC */
 void msc_dtap(struct gsm_subscriber_connection *conn, uint8_t link_id, struct msgb *msg)
 {
+	msc_conn_get(conn);
 	gsm0408_dispatch(conn, msg);
 
 	/* Bump whether the conn wants to be closed */
 	subscr_conn_bump(conn);
+	msc_conn_put(conn);
 }
 
 /* Receive an ASSIGNMENT COMPLETE from BSC */
@@ -203,7 +210,6 @@ void msc_subscr_con_cleanup(struct gsm_subscriber_connection *conn)
 				? OSMO_FSM_TERM_REGULAR
 				: OSMO_FSM_TERM_ERROR,
 			   NULL);
-	conn->conn_fsm = NULL;
 }
 
 void msc_subscr_con_free(struct gsm_subscriber_connection *conn)
@@ -241,20 +247,17 @@ void msc_close_connection(struct gsm_subscriber_connection *conn)
 		return;
 	if (conn->in_release)
 		return;
-	if (!conn->conn_fsm) {
-		/* No FSM means no valid process is ongoing. Discard right
-		 * away. */
-		msc_subscr_con_free(conn);
+	if (!conn->conn_fsm)
 		return;
-	}
 	if (conn->conn_fsm->state == SUBSCR_CONN_S_RELEASED)
 		return;
 	osmo_fsm_inst_dispatch(conn->conn_fsm, SUBSCR_CONN_E_CN_CLOSE, NULL);
 }
 
 /* increment the ref-count. Needs to be called by every user */
-struct gsm_subscriber_connection *_subscr_con_get(struct gsm_subscriber_connection *conn,
-						  const char *file, int line)
+struct gsm_subscriber_connection *
+_msc_conn_get(struct gsm_subscriber_connection *conn,
+	      const char *file, int line)
 {
 	OSMO_ASSERT(conn);
 
@@ -262,33 +265,33 @@ struct gsm_subscriber_connection *_subscr_con_get(struct gsm_subscriber_connecti
 		return NULL;
 
 	conn->use_count++;
-	LOGPSRC(DMSC, LOGL_DEBUG, file, line,
-		"subscr %s: increased subscr_con use_count to %u\n",
+	LOGPSRC(DREF, LOGL_DEBUG, file, line,
+		"%s: MSC conn use + 1 == %u\n",
 		vlr_subscr_name(conn->vsub), conn->use_count);
 
 	return conn;
 }
 
 /* decrement the ref-count. Once it reaches zero, we release */
-void _subscr_con_put(struct gsm_subscriber_connection *conn,
-		     const char *file, int line)
+void _msc_conn_put(struct gsm_subscriber_connection *conn,
+		   const char *file, int line)
 {
 	OSMO_ASSERT(conn);
 
 	if (conn->use_count == 0) {
-		LOGP(DMSC, LOGL_ERROR, "trying to decrement conn use count, but is alrady 0\n");
+		LOGPSRC(DREF, LOGL_ERROR, file, line,
+			"%s: MSC conn use - 1 failed: is already 0\n",
+			vlr_subscr_name(conn->vsub));
 		return;
 	}
 
 	conn->use_count--;
-	LOGPSRC(DMSC, LOGL_DEBUG, file, line,
-		"subscr %s: decreased subscr_conn use_count to %u\n",
+	LOGPSRC(DREF, LOGL_DEBUG, file, line,
+		"%s: MSC conn use - 1 == %u\n",
 		vlr_subscr_name(conn->vsub), conn->use_count);
 
-#if 0
-	if (conn->use_count == 0 && conn->conn_fsm)
-		osmo_fsm_inst_dispatch(conn->conn_fsm, SUBSCR_CONN_E_MO_CLOSE, NULL);
-#endif
+	if (conn->use_count == 0)
+		msc_subscr_con_free(conn);
 }
 
 void msc_stop_paging(struct vlr_subscr *vsub)
diff --git a/openbsc/src/libmsc/silent_call.c b/openbsc/src/libmsc/silent_call.c
index 7027dce11..f519fbb8c 100644
--- a/openbsc/src/libmsc/silent_call.c
+++ b/openbsc/src/libmsc/silent_call.c
@@ -57,7 +57,7 @@ static int paging_cb_silent(unsigned int hooknum, unsigned int event,
 			conn->lchan->ts->nr, conn->lchan->ts->trx->arfcn);
 #endif
 		conn->silent_call = 1;
-		subscr_con_get(conn);
+		msc_conn_get(conn);
 		/* increment lchan reference count */
 		osmo_signal_dispatch(SS_SCALL, S_SCALL_SUCCESS, &sigdata);
 		break;
@@ -154,7 +154,7 @@ int gsm_silent_call_stop(struct vlr_subscr *vsub)
 #endif
 
 	conn->silent_call = 0;
-	subscr_con_put(conn);
+	msc_conn_put(conn);
 
 	return 0;
 }
diff --git a/openbsc/src/libmsc/subscr_conn.c b/openbsc/src/libmsc/subscr_conn.c
index 6eeb2dba3..41414f28f 100644
--- a/openbsc/src/libmsc/subscr_conn.c
+++ b/openbsc/src/libmsc/subscr_conn.c
@@ -231,15 +231,7 @@ static void subscr_conn_fsm_cleanup(struct osmo_fsm_inst *fi,
 	conn->in_release = true;
 	conn->conn_fsm = NULL;
 
-	/* If we're closing in a middle of a trans, we need to clean up */
-	if (conn->use_count) {
-		DEBUGP(DMM, "%s: still in use (%u), cleaning up transactions\n",
-		       vlr_subscr_name(conn->vsub), conn->use_count);
-		trans_conn_closed(conn);
-	}
-	if (conn->use_count)
-		LOGP(DMM, LOGL_ERROR, "%s: closing conn but still in use (%u)\n",
-		     vlr_subscr_name(conn->vsub), conn->use_count);
+	trans_conn_closed(conn);
 
 	if (conn->via_ran == RAN_UTRAN_IU)
 		iu_tx_release(conn->iu.ue_ctx, NULL);
@@ -247,7 +239,7 @@ static void subscr_conn_fsm_cleanup(struct osmo_fsm_inst *fi,
 		 * received from the UE, or a timeout expires. For now, the log
 		 * says "unknown UE" for each release outcome. */
 
-	msc_subscr_con_free(conn);
+	msc_conn_put(conn);
 }
 
 int subscr_conn_fsm_timeout(struct osmo_fsm_inst *fi)
@@ -340,8 +332,8 @@ int msc_create_conn_fsm(struct gsm_subscriber_connection *conn, const char *id)
 	 * subscriber connection. If the FSM is freed along with the subscriber
 	 * connection, then in _osmo_fsm_inst_term() the osmo_fsm_inst_free()
 	 * that follows the cleanup() call would run into a double free. */
-	fi = osmo_fsm_inst_alloc(&subscr_conn_fsm, conn->network, conn,
-				 LOGL_DEBUG, id);
+	fi = osmo_fsm_inst_alloc(&subscr_conn_fsm, conn->network,
+				 msc_conn_get(conn), LOGL_DEBUG, id);
 
 	if (!fi) {
 		LOGP(DMM, LOGL_ERROR,
diff --git a/openbsc/src/libmsc/transaction.c b/openbsc/src/libmsc/transaction.c
index af9b30003..2c205af95 100644
--- a/openbsc/src/libmsc/transaction.c
+++ b/openbsc/src/libmsc/transaction.c
@@ -132,7 +132,7 @@ void trans_free(struct gsm_trans *trans)
 	llist_del(&trans->entry);
 
 	if (trans->conn)
-		subscr_con_put(trans->conn);
+		msc_conn_put(trans->conn);
 
 	trans->conn = NULL;
 	talloc_free(trans);