ms: Hold a reference during ms_alloc
Make the caller hold a reference to the MS object just allocated, so
that it hs to explicitly unref it and, in turn, if no new references
were added during its use, trigger release of the MS object.
This is useful to avoid leaking MS object if it was allocated and then
no TBF is attached to it because allocation of TBF failed.
Related: OS#6002
Change-Id: I2088a7ddd76fe9157b6626ef96ae4315e88779ea
diff --git a/src/bts.cpp b/src/bts.cpp
index 40541c5..4cbe68e 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -1006,9 +1006,13 @@
"SBFn=%u TRX=%u TS=%u\n", sb_fn, pdch->trx->trx_no, pdch->ts_no);
bts_do_rate_ctr_inc(bts, CTR_IMMEDIATE_ASSIGN_UL_TBF_TWO_PHASE);
} else {
- GprsMs *ms = ms_alloc(bts);
+ GprsMs *ms = ms_alloc(bts, __func__);
ms_set_egprs_ms_class(ms, chan_req.egprs_mslot_class);
tbf = ms_new_ul_tbf_assigned_agch(ms);
+ /* Here either tbf was created and it holds a ref to MS, or tbf
+ * creation failed and MS will end up without references and being
+ * freed: */
+ ms_unref(ms, __func__);
if (!tbf) {
/* Send RR Immediate Assignment Reject */
rc = -EBUSY;
diff --git a/src/gprs_ms.c b/src/gprs_ms.c
index 6d7018f..7df71e8 100644
--- a/src/gprs_ms.c
+++ b/src/gprs_ms.c
@@ -122,7 +122,7 @@
}
static int ms_talloc_destructor(struct GprsMs *ms);
-struct GprsMs *ms_alloc(struct gprs_rlcmac_bts *bts)
+struct GprsMs *ms_alloc(struct gprs_rlcmac_bts *bts, const char *use_ref)
{
struct GprsMs *ms = talloc_zero(tall_pcu_ctx, struct GprsMs);
OSMO_ASSERT(bts);
@@ -172,6 +172,8 @@
ms_set_timeout(ms, osmo_tdef_get(bts->pcu->T_defs, -2030, OSMO_TDEF_S, -1));
+ if (use_ref)
+ ms_ref(ms, use_ref);
return ms;
free_ret:
talloc_free(ms);
diff --git a/src/gprs_ms.h b/src/gprs_ms.h
index cdb23bf..03508ff 100644
--- a/src/gprs_ms.h
+++ b/src/gprs_ms.h
@@ -95,7 +95,7 @@
struct nacc_fsm_ctx *nacc;
};
-struct GprsMs *ms_alloc(struct gprs_rlcmac_bts *bts);
+struct GprsMs *ms_alloc(struct gprs_rlcmac_bts *bts, const char *use_ref);
struct gprs_rlcmac_pdch *ms_first_common_ts(const struct GprsMs *ms);
void ms_set_first_common_ts(struct GprsMs *ms, struct gprs_rlcmac_pdch *pdch);
diff --git a/src/pdch.cpp b/src/pdch.cpp
index 4495e21..a8becca 100644
--- a/src/pdch.cpp
+++ b/src/pdch.cpp
@@ -665,7 +665,9 @@
uint32_t tlli = request->ID.u.TLLI;
ms = bts_get_ms_by_tlli(bts, tlli, GSM_RESERVED_TMSI);
if (!ms) {
- ms = ms_alloc(bts);
+ /* A reference is already hold immediately below in all cases, no
+ * need to hold and extra one, pass use_ref=NULL: */
+ ms = ms_alloc(bts, NULL);
ms_set_tlli(ms, tlli);
}
} else if (request->ID.u.Global_TFI.UnionType) { /* ID_TYPE = DL_TFI */
@@ -852,13 +854,16 @@
struct gprs_rlcmac_sba *sba;
struct pdch_ulc_node *poll;
GprsMs *ms;
+ bool ms_allocated = false;
ms = bts_get_ms_by_tlli(bts(), report->TLLI, GSM_RESERVED_TMSI);
if (!ms) {
LOGPDCH(this, DRLCMAC, LOGL_NOTICE, "MS send measurement "
"but TLLI 0x%08x is unknown\n", report->TLLI);
- ms = ms_alloc(bts());
+ ms = ms_alloc(bts(), __func__);
ms_set_tlli(ms, report->TLLI);
+ /* Track that we need to free ref at the end: */
+ ms_allocated = true;
}
if ((poll = pdch_ulc_get_node(ulc, fn))) {
switch (poll->type) {
@@ -879,6 +884,8 @@
}
}
gprs_rlcmac_meas_rep(ms, report);
+ if (ms_allocated)
+ ms_unref(ms, __func__);
}
void gprs_rlcmac_pdch::rcv_cell_change_notification(Packet_Cell_Change_Notification_t *notif,
diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index 628d593..17b6a3a 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -201,27 +201,31 @@
{
int rc;
GprsMs *ms, *ms_old;
+ bool ms_allocated = false;
/* check for existing TBF */
ms = bts_get_ms(bts, tlli, tlli_old, imsi);
/* If we got MS by TLLI above let's see if we already have another MS
* object identified by IMSI and merge them */
- if (ms && !ms_imsi_is_valid(ms) && imsi) {
- ms_old = bts_get_ms_by_imsi(bts, imsi);
- if (ms_old && ms_old != ms) {
- /* The TLLI has changed (RAU), so there are two MS
- * objects for the same MS */
- LOGP(DTBF, LOGL_NOTICE,
- "There is a new MS object for the same MS: (0x%08x, '%s') -> (0x%08x, '%s')\n",
- ms_tlli(ms_old), ms_imsi(ms_old), ms_tlli(ms), ms_imsi(ms));
- ms_merge_and_clear_ms(ms, ms_old);
- /* old_ms may no longer be available here */
+ if (ms) {
+ if (!ms_imsi_is_valid(ms) && imsi) {
+ ms_old = bts_get_ms_by_imsi(bts, imsi);
+ if (ms_old && ms_old != ms) {
+ /* The TLLI has changed (RAU), so there are two MS
+ * objects for the same MS */
+ LOGP(DTBF, LOGL_NOTICE,
+ "There is a new MS object for the same MS: (0x%08x, '%s') -> (0x%08x, '%s')\n",
+ ms_tlli(ms_old), ms_imsi(ms_old), ms_tlli(ms), ms_imsi(ms));
+ ms_merge_and_clear_ms(ms, ms_old);
+ /* old_ms may no longer be available here */
+ }
}
+ } else {
+ ms = ms_alloc(bts, __func__);
+ /* Remember we have to unref the alloc reference at the end: */
+ ms_allocated = true;
}
-
- if (!ms)
- ms = ms_alloc(bts);
if (imsi)
ms_set_imsi(ms, imsi);
ms_confirm_tlli(ms, tlli);
@@ -233,9 +237,12 @@
}
rc = ms_append_llc_dl_data(ms, delay_csec, data, len);
- if (rc < 0)
- return rc;
- return 0;
+ if (ms_allocated) {
+ ms_unref(ms, __func__);
+ /* Here "ms" may be freed if ms_append_llc_dl_data() failed to
+ * allocate a DL TBF and it has no more TBFs attached */
+ }
+ return rc;
}
bool gprs_rlcmac_dl_tbf::restart_bsn_cycle()