ms: store in bts->ms_list during alloc/destroy of ms object

With this change the MS no longer is removed from the llist without
potentially skipping free() if not idle in bts_ms_idle_cb().
As a result, some unit tests now can free it during bts tear down
instead of having them leaked.
The tests int MsTest need changes because the tbfs created are fake and
cannot be freed using tbf_free(), and hence cannot be detached from MS
using regular code paths. Instead first call explicit talloc_free(ms)
like other unit tests in the file already do.

Change-Id: Id53f8dfb9963366dd4b19a324615bbc83c4f23e7

5 files changed, 15 insertions, 4 deletions

diff --git a/src/bts.cpp b/src/bts.cpp
index 84eea088..3b27676d 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -244,7 +244,6 @@ static int bts_talloc_destructor(struct gprs_rlcmac_bts* bts)
 	while ((ms = llist_first_entry_or_null(&bts->ms_list, struct GprsMs, list))) {
 		ms_set_callback(ms, NULL);
 		ms_set_timeout(ms, 0);
-		llist_del(&ms->list);
 		bts_stat_item_dec(bts, STAT_MS_PRESENT);
 		talloc_free(ms);
 	}
@@ -1201,7 +1200,6 @@ bool bts_cs_dl_is_supported(const struct gprs_rlcmac_bts* bts, CodingScheme cs)
 
 static void bts_ms_idle_cb(struct GprsMs *ms)
 {
-	llist_del(&ms->list);
 	bts_stat_item_dec(ms->bts, STAT_MS_PRESENT);
 	if (ms_is_idle(ms))
 		talloc_free(ms);
@@ -1225,7 +1223,6 @@ GprsMs *bts_alloc_ms(struct gprs_rlcmac_bts* bts)
 
 	ms_set_callback(ms, &bts_ms_cb);
 	ms_set_timeout(ms, osmo_tdef_get(bts->pcu->T_defs, -2030, OSMO_TDEF_S, -1));
-	llist_add(&ms->list, &bts->ms_list);
 
 	bts_stat_item_inc(bts, STAT_MS_PRESENT);
 	return ms;
diff --git a/src/gprs_ms.c b/src/gprs_ms.c
index 7ee697e9..3e95103d 100644
--- a/src/gprs_ms.c
+++ b/src/gprs_ms.c
@@ -110,6 +110,8 @@ struct GprsMs *ms_alloc(struct gprs_rlcmac_bts *bts)
 
 	talloc_set_destructor(ms, ms_talloc_destructor);
 
+	llist_add(&ms->list, &bts->ms_list);
+
 	ms->bts = bts;
 	ms->cb = gprs_default_cb;
 	ms->tlli = GSM_RESERVED_TMSI;
@@ -119,7 +121,6 @@ struct GprsMs *ms_alloc(struct gprs_rlcmac_bts *bts)
 	ms->current_cs_ul = UNKNOWN;
 	ms->current_cs_dl = UNKNOWN;
 	ms->is_idle = true;
-	INIT_LLIST_HEAD(&ms->list);
 	INIT_LLIST_HEAD(&ms->old_tbfs);
 
 	int codel_interval = LLC_CODEL_USE_DEFAULT;
@@ -158,6 +159,8 @@ static int ms_talloc_destructor(struct GprsMs *ms)
 
 	LOGPMS(ms, DRLCMAC, LOGL_INFO, "Destroying MS object\n");
 
+	llist_del(&ms->list);
+
 	ms_set_reserved_slots(ms, NULL, 0, 0);
 
 	osmo_timer_del(&ms->timer);
diff --git a/tests/ms/MsTest.cpp b/tests/ms/MsTest.cpp
index 45051d4a..84cb52e8 100644
--- a/tests/ms/MsTest.cpp
+++ b/tests/ms/MsTest.cpp
@@ -457,6 +457,7 @@ static void test_ms_storage()
 	ms = bts_get_ms_by_tlli(bts, tlli + 1, GSM_RESERVED_TMSI);
 	OSMO_ASSERT(ms == NULL);
 
+	talloc_free(ms);
 	talloc_free(ul_tbf);
 	talloc_free(bts);
 	printf("=== end %s ===\n", __func__);
@@ -548,6 +549,7 @@ static void test_ms_cs_selection()
 
 	OSMO_ASSERT(mcs_chan_code(ms_current_cs_dl(ms, ms_mode(ms))) == 2);
 
+	talloc_free(ms);
 	talloc_free(dl_tbf);
 	talloc_free(bts);
 	printf("=== end %s ===\n", __func__);
@@ -617,6 +619,8 @@ static void test_ms_mcs_mode()
 	ms_set_mode(ms2, GPRS);
 	dump_ms(ms2, "2: after mode set   ");
 
+	talloc_free(ms1);
+	talloc_free(ms2);
 	talloc_free(dl_tbf);
 	talloc_free(bts);
 	printf("=== end %s ===\n", __func__);
diff --git a/tests/ms/MsTest.err b/tests/ms/MsTest.err
index 218ae59a..d6677400 100644
--- a/tests/ms/MsTest.err
+++ b/tests/ms/MsTest.err
@@ -61,8 +61,13 @@ MS(TLLI-0xffeeddbb:TA-220:MSCLS-0-0) Destroying MS object
 Creating MS object
 The MS object cannot fully confirm an unexpected TLLI: 0xffeeddbb, partly confirmed
 MS(TLLI-0xffeeddbb:TA-220:MSCLS-0-0) Attaching DL TBF: TBF(DL:STATE-NEW:GPRS:TLLI-0xffeeddbb)
+MS(TLLI-0xffeeddbb:TA-220:MSCLS-0-0:DL) Destroying MS object
+MS(TLLI-0xffeeddbb:TA-220:MSCLS-0-0) Detaching TBF: TBF(DL:STATE-NEW:GPRS:TLLI-0xffeeddbb)
 Creating MS object
 The MS object cannot fully confirm an unexpected TLLI: 0xdeadbeef, partly confirmed
 Creating MS object
 The MS object cannot fully confirm an unexpected TLLI: 0xdeadbef0, partly confirmed
 MS(TLLI-0xdeadbef0:TA-220:MSCLS-0-0) Attaching DL TBF: TBF(DL:STATE-NEW:GPRS:TLLI-0xdeadbef0)
+MS(TLLI-0xdeadbeef:TA-220:MSCLS-0-0) Destroying MS object
+MS(TLLI-0xdeadbef0:TA-220:MSCLS-0-0:DL) Destroying MS object
+MS(TLLI-0xdeadbef0:TA-220:MSCLS-0-0) Detaching TBF: TBF(DL:STATE-NEW:GPRS:TLLI-0xdeadbef0)
diff --git a/tests/ulc/PdchUlcTest.err b/tests/ulc/PdchUlcTest.err
index a2ec373f..41aa9b4b 100644
--- a/tests/ulc/PdchUlcTest.err
+++ b/tests/ulc/PdchUlcTest.err
@@ -72,6 +72,8 @@ PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=34 is still reserved!
 PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=34, reason=UL_ASS): TBF(DL:TFI-0-0--1:STATE-NEW:GPRS:TLLI-0x12345678)
 PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=39 is still reserved!
 PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=39, reason=UL_ASS): TBF(DL:TFI-0-0--1:STATE-NEW:GPRS:TLLI-0x12345678)
+MS(TLLI-0x12345678:TA-220:MSCLS-0-0:DL) Destroying MS object
+MS(TLLI-0x12345678:TA-220:MSCLS-0-0) Detaching TBF: TBF(DL:TFI-0-0--1:STATE-NEW:GPRS:TLLI-0x12345678)
 PDCH(bts=0,trx=0,ts=0) PDCH state: disabled => enabled
 PDCH(bts=0,trx=0,ts=0) Reserving FN 104 for type SBA
 PDCH(bts=0,trx=0,ts=0) Reserving FN 117 for type SBA