Make sure GprsMs free() also frees its tbfs

This fixes TBF objects leaking and ending up alive when the MS object is
explicitly freed through talloc_free (and sporadically
crashing TbfTest once a timeout for them occur).

This mostly affects unit tests, where most of the explicit free()
happens.
In osmo-pcu, in general, the GprsMs object only gets _free() called when
its resource count reaches 0, aka no more TBFs are attached to it. Hence
in general GprsMs object is freed() only when no TBFs (to be leaked) are
present.
However, in the unit tests it's usual that we want to wipe the entire
context by eg. feeing the PCU, the BTS or MS object, which should also
free the related TBFs.

When running osmo-pcu this may only be an issue when the MS object is
freed explicitly, which could happen for instance when a BTS is torn down,
ie. PCUIF going down, moment at which all GprsMs of that BTS are freed.
But in there actually it iterates over PDCHs to free all TBFs, so it's
fine.
If we iterated over MS, this could have ended up in a crash, like
it happened in TbfTest sporadically, but it's not a bit problem if we
crash + restart at that time since anyway the BTS is gone ore just
getting up around that time.

Related: OS#6359
Change-Id: Ibbdec94acb8132be20508d3178d88da44bfaf91d

3 files changed, 12 insertions, 12 deletions

diff --git a/tests/ms/MsTest.cpp b/tests/ms/MsTest.cpp
index f0514017..5bfbc28f 100644
--- a/tests/ms/MsTest.cpp
+++ b/tests/ms/MsTest.cpp
@@ -443,8 +443,6 @@ static void test_ms_cs_selection()
 	dl_tbf = alloc_dl_tbf(bts, ms);
 	ms_attach_tbf(ms, dl_tbf);
 
-	ms_unref(ms, __func__);
-
 	OSMO_ASSERT(!ms_is_idle(ms));
 
 	OSMO_ASSERT(mcs_chan_code(ms_current_cs_dl(ms, ms_mode(ms))) == 3);
@@ -453,8 +451,10 @@ static void test_ms_cs_selection()
 
 	OSMO_ASSERT(mcs_chan_code(ms_current_cs_dl(ms, ms_mode(ms))) == 2);
 
-	talloc_free(ms);
+	ms_detach_tbf(ms, dl_tbf);
 	talloc_free(dl_tbf);
+	ms_unref(ms, __func__);
+	/* MS has been freed here*/
 	talloc_free(bts);
 	printf("=== end %s ===\n", __func__);
 }
@@ -513,9 +513,8 @@ static void test_ms_mcs_mode()
 	ms_set_mode(ms2, EGPRS_GMSK);
 	dump_ms(ms2, "2: after mode set   ");
 
-	// FIXME: following code triggers ASAN failure:
-	// ms2->detach_tbf(dl_tbf);
-	// dump_ms(ms2, "2: after TBF detach ");
+	ms_detach_tbf(ms2, dl_tbf);
+	dump_ms(ms2, "2: after TBF detach ");
 
 	ms_set_mode(ms1, GPRS);
 	dump_ms(ms1, "1: after mode set   ");
@@ -523,9 +522,9 @@ static void test_ms_mcs_mode()
 	ms_set_mode(ms2, GPRS);
 	dump_ms(ms2, "2: after mode set   ");
 
+	talloc_free(dl_tbf);
 	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 eb3c72e9..001898b9 100644
--- a/tests/ms/MsTest.err
+++ b/tests/ms/MsTest.err
@@ -68,9 +68,9 @@ Creating MS object
 MS(TA-220:MSCLS-0-0): + test_ms_cs_selection: now used by 1 (test_ms_cs_selection)
 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:G:TLLI-0xffeeddbb){NEW}
-MS(TLLI-0xffeeddbb:TA-220:MSCLS-0-0:DL) Destroying MS object
 MS(TLLI-0xffeeddbb:TA-220:MSCLS-0-0:DL) Detaching TBF: TBF(DL:G:TLLI-0xffeeddbb){NEW}
-MS(TLLI-0xffeeddbb:TA-220:MSCLS-0-0): - tbf: now used by 0 (-)
+MS(TLLI-0xffeeddbb:TA-220:MSCLS-0-0): - test_ms_cs_selection: now used by 0 (-)
+MS(TLLI-0xffeeddbb:TA-220:MSCLS-0-0) Destroying MS object
 Creating MS object
 MS(TA-220:MSCLS-0-0): + test_ms_mcs_mode: now used by 1 (test_ms_mcs_mode)
 The MS object cannot fully confirm an unexpected TLLI: 0xdeadbeef, partly confirmed
@@ -78,6 +78,6 @@ Creating MS object
 MS(TA-220:MSCLS-0-0): + test_ms_mcs_mode: now used by 1 (test_ms_mcs_mode)
 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:G:TLLI-0xdeadbef0){NEW}
-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:DL) Detaching TBF: TBF(DL:G:TLLI-0xdeadbef0){NEW}
+MS(TLLI-0xdeadbeef:TA-220:MSCLS-0-0) Destroying MS object
+MS(TLLI-0xdeadbef0:TA-220:MSCLS-0-0) Destroying MS object
diff --git a/tests/ms/MsTest.ok b/tests/ms/MsTest.ok
index 331bef0d..a79dedb1 100644
--- a/tests/ms/MsTest.ok
+++ b/tests/ms/MsTest.ok
@@ -20,6 +20,7 @@
 2: after MCS set     MS DL MCS-8/MCS-9, UL MCS-1/MCS-9, mode EGPRS, <ACTIVE>
 1: after mode set    MS DL MCS-1/MCS-4, UL MCS-1/MCS-4, mode EGPRS_GMSK-only, <IDLE>
 2: after mode set    MS DL MCS-1/MCS-4, UL MCS-1/MCS-4, mode EGPRS_GMSK-only, <ACTIVE>
+2: after TBF detach  MS DL MCS-1/MCS-4, UL MCS-1/MCS-4, mode EGPRS_GMSK-only, <IDLE>
 1: after mode set    MS DL CS-4/CS-4, UL CS-1/CS-4, mode GPRS, <IDLE>
-2: after mode set    MS DL CS-4/CS-4, UL CS-1/CS-4, mode GPRS, <ACTIVE>
+2: after mode set    MS DL CS-4/CS-4, UL CS-1/CS-4, mode GPRS, <IDLE>
 === end test_ms_mcs_mode ===