pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcu/+/32346 ) Change subject: ms: store in bts->ms_list during alloc/destroy of ms object ...................................................................... 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 --- M src/bts.cpp M src/gprs_ms.c M tests/ms/MsTest.cpp M tests/ms/MsTest.err M tests/ulc/PdchUlcTest.err 5 files changed, 33 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/46/32346/1 diff --git a/src/bts.cpp b/src/bts.cpp index 84eea08..3b27676 100644 --- a/src/bts.cpp +++ b/src/bts.cpp @@ -244,7 +244,6 @@ 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 @@ 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 @@ 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 b48961b..48867fd 100644 --- a/src/gprs_ms.c +++ b/src/gprs_ms.c @@ -110,6 +110,8 @@ 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 @@ 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 @@ 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 45051d4..84cb52e 100644 --- a/tests/ms/MsTest.cpp +++ b/tests/ms/MsTest.cpp @@ -457,6 +457,7 @@ 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 @@ 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 @@ 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 218ae59..d667740 100644 --- a/tests/ms/MsTest.err +++ b/tests/ms/MsTest.err @@ -61,8 +61,13 @@ 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 a2ec373..41aa9b4 100644 --- a/tests/ulc/PdchUlcTest.err +++ b/tests/ulc/PdchUlcTest.err @@ -72,6 +72,8 @@ 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 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/32346 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Id53f8dfb9963366dd4b19a324615bbc83c4f23e7 Gerrit-Change-Number: 32346 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin@sysmocom.de> Gerrit-MessageType: newchange