trans_free: tear down conn when last transaction is done

In trans_free(), call subscr_conn_release_when_unused(), so that we are sure to
clean up after the last transaction is done.

This fixes an error where a conn lingered after a CC failure, because that code
path forgot to trigger cleanup.

Rationale: so far we were triggering the release check after each DTAP dispatch
(compl_l3 and "normal" DTAP), which is sufficient for properly closed
transactions. We also need a check for when a timeout clears an erratic trans.

Adjust test expectation of test_call_mo_to_unknown_timeout to show that the
error is now fixed.

msc_vlr_test_reject_concurrency now sees an additional release checking event
when the SMS transaction is done, which is expected and does not affect the
test otherwise.

Related: OS#2779
Change-Id: I46ff2e9b09b67e4e0d79cccf8c04936f17281fcb

3 files changed, 14 insertions, 3 deletions

diff --git a/src/libmsc/osmo_msc.c b/src/libmsc/osmo_msc.c
index 01e44f37f..d0c523502 100644
--- a/src/libmsc/osmo_msc.c
+++ b/src/libmsc/osmo_msc.c
@@ -47,7 +47,7 @@ void msc_sapi_n_reject(struct gsm_subscriber_connection *conn, int dlci)
 		gsm411_sapi_n_reject(conn);
 }
 
-static void subscr_conn_release_when_unused(struct gsm_subscriber_connection *conn)
+void subscr_conn_release_when_unused(struct gsm_subscriber_connection *conn)
 {
 	if (!conn)
 		return;
diff --git a/src/libmsc/subscr_conn.c b/src/libmsc/subscr_conn.c
index 8c424c536..ca93a0369 100644
--- a/src/libmsc/subscr_conn.c
+++ b/src/libmsc/subscr_conn.c
@@ -371,8 +371,11 @@ bool msc_subscr_conn_is_accepted(const struct gsm_subscriber_connection *conn)
 void msc_subscr_conn_communicating(struct gsm_subscriber_connection *conn)
 {
 	OSMO_ASSERT(conn);
-	osmo_fsm_inst_dispatch(conn->conn_fsm, SUBSCR_CONN_E_COMMUNICATING,
-			       NULL);
+	/* This function is called to indicate that *some* communication is happening with the phone.
+	 * Late in the process, that may be a Release Confirm and the FSM and conn are already in
+	 * teardown. No need to signal SUBSCR_CONN_E_COMMUNICATING then. */
+	if (conn->conn_fsm)
+		osmo_fsm_inst_dispatch(conn->conn_fsm, SUBSCR_CONN_E_COMMUNICATING, NULL);
 }
 
 void msc_subscr_conn_init(void)
diff --git a/src/libmsc/transaction.c b/src/libmsc/transaction.c
index cdaba9c79..4a3b064db 100644
--- a/src/libmsc/transaction.c
+++ b/src/libmsc/transaction.c
@@ -117,6 +117,7 @@ struct gsm_trans *trans_alloc(struct gsm_network *net,
 void trans_free(struct gsm_trans *trans)
 {
 	enum msc_subscr_conn_use conn_usage_token = MSC_CONN_USE_UNTRACKED;
+	struct gsm_subscriber_connection *conn;
 
 	switch (trans->protocol) {
 	case GSM48_PDISC_CC:
@@ -153,8 +154,15 @@ void trans_free(struct gsm_trans *trans)
 	if (trans->conn)
 		msc_subscr_conn_put(trans->conn, conn_usage_token);
 
+	conn = trans->conn;
 	trans->conn = NULL;
 	talloc_free(trans);
+
+	/* trans_free() should always happen while the conn_fsm is still around. */
+	OSMO_ASSERT(conn->conn_fsm);
+
+	/* Possibly this was the last transaction used by this conn. */
+	subscr_conn_release_when_unused(conn);
 }
 
 /*! allocate an unused transaction ID for the given subscriber