<p>osmith has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/25217">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">bts: delete pch_timer list in destructor<br><br>Run bts_pch_timer_remove() on each entry of the BTS specific pch_timer<br>list, so we don't have a memory leak and so the timer doesn't<br>potentially fire for a deallocated BTS.<br><br>Fixes: d3c7591 ("Add counters: pcu.bts.N.pch.requests.timeout")<br>Change-Id: Ia5e33d1894408e93a51c452002ef2f5758808269<br>---<br>M src/bts.cpp<br>M src/bts_pch_timer.c<br>M src/bts_pch_timer.h<br>M tests/alloc/AllocTest.cpp<br>M tests/alloc/AllocTest.err<br>5 files changed, 40 insertions(+), 0 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/17/25217/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/bts.cpp b/src/bts.cpp</span><br><span>index a40e071..daa8ee1 100644</span><br><span>--- a/src/bts.cpp</span><br><span>+++ b/src/bts.cpp</span><br><span>@@ -32,6 +32,7 @@</span><br><span> #include <pdch.h></span><br><span> #include <gprs_ms_storage.h></span><br><span> #include <sba.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <bts_pch_timer.h></span><br><span> </span><br><span> extern "C" {</span><br><span>       #include <osmocom/core/talloc.h></span><br><span>@@ -230,6 +231,8 @@</span><br><span>                 bts->app_info = NULL;</span><br><span>     }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ bts_pch_timer_stop_all(bts);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>       llist_del(&bts->list);</span><br><span>        return 0;</span><br><span> }</span><br><span>diff --git a/src/bts_pch_timer.c b/src/bts_pch_timer.c</span><br><span>index 386a583..20373ac 100644</span><br><span>--- a/src/bts_pch_timer.c</span><br><span>+++ b/src/bts_pch_timer.c</span><br><span>@@ -83,3 +83,12 @@</span><br><span>         if (p)</span><br><span>               bts_pch_timer_remove(p);</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+void bts_pch_timer_stop_all(struct gprs_rlcmac_bts *bts)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+  struct bts_pch_timer *p, *n;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+        llist_for_each_entry_safe(p, n, &bts->pch_timer, entry) {</span><br><span style="color: hsl(120, 100%, 40%);">+              bts_pch_timer_remove(p);</span><br><span style="color: hsl(120, 100%, 40%);">+      }</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span>diff --git a/src/bts_pch_timer.h b/src/bts_pch_timer.h</span><br><span>index 91bebed..26b89c8 100644</span><br><span>--- a/src/bts_pch_timer.h</span><br><span>+++ b/src/bts_pch_timer.h</span><br><span>@@ -37,6 +37,7 @@</span><br><span> </span><br><span> void bts_pch_timer_start(struct gprs_rlcmac_bts *bts, const char *imsi);</span><br><span> void bts_pch_timer_stop(struct gprs_rlcmac_bts *bts, const char *imsi);</span><br><span style="color: hsl(120, 100%, 40%);">+void bts_pch_timer_stop_all(struct gprs_rlcmac_bts *bts);</span><br><span> </span><br><span> #ifdef __cplusplus</span><br><span> }</span><br><span>diff --git a/tests/alloc/AllocTest.cpp b/tests/alloc/AllocTest.cpp</span><br><span>index 5f11ec5..2132783 100644</span><br><span>--- a/tests/alloc/AllocTest.cpp</span><br><span>+++ b/tests/alloc/AllocTest.cpp</span><br><span>@@ -24,6 +24,7 @@</span><br><span> #include "tbf_dl.h"</span><br><span> #include "bts.h"</span><br><span> #include "gprs_ms.h"</span><br><span style="color: hsl(120, 100%, 40%);">+#include "bts_pch_timer.h"</span><br><span> </span><br><span> #include <string.h></span><br><span> #include <stdio.h></span><br><span>@@ -802,6 +803,24 @@</span><br><span>       talloc_free(bts);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static void test_bts_pch_timer(void)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+   struct gprs_rlcmac_bts *bts = bts_alloc(the_pcu, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+  const char *imsi1 = "1234";</span><br><span style="color: hsl(120, 100%, 40%);">+ const char *imsi2 = "5678";</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+       fprintf(stderr, "Testing bts_pch_timer dealloc on bts dealloc\n");</span><br><span style="color: hsl(120, 100%, 40%);">+  log_set_category_filter(osmo_stderr_target, DPCU, 1, LOGL_DEBUG);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   fprintf(stderr, "Starting PCH timer for 2 IMSI\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ bts_pch_timer_start(bts, imsi1);</span><br><span style="color: hsl(120, 100%, 40%);">+      bts_pch_timer_start(bts, imsi2);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+    /* pch timer for imsi2 must get removed here */</span><br><span style="color: hsl(120, 100%, 40%);">+       fprintf(stderr, "Deallocating BTS, expecting the PCH timer to be stopped and deallocated\n");</span><br><span style="color: hsl(120, 100%, 40%);">+       talloc_free(bts);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> int main(int argc, char **argv)</span><br><span> {</span><br><span>       tall_pcu_ctx = talloc_named_const(NULL, 1, "moiji-mobile AllocTest context");</span><br><span>@@ -828,6 +847,7 @@</span><br><span>        test_many_connections(alloc_algorithm_b, 32, "B");</span><br><span>         test_many_connections(alloc_algorithm_dynamic, 160, "dynamic");</span><br><span>    test_2_consecutive_dl_tbfs();</span><br><span style="color: hsl(120, 100%, 40%);">+ test_bts_pch_timer();</span><br><span> </span><br><span>    talloc_free(the_pcu);</span><br><span>        return EXIT_SUCCESS;</span><br><span>diff --git a/tests/alloc/AllocTest.err b/tests/alloc/AllocTest.err</span><br><span>index 4ae5e56..afb84bf 100644</span><br><span>--- a/tests/alloc/AllocTest.err</span><br><span>+++ b/tests/alloc/AllocTest.err</span><br><span>@@ -326119,3 +326119,10 @@</span><br><span> TBF(DL-TFI_1){NULL}: state_chg to RELEASING</span><br><span> TBF(TFI=1 TLLI=0xffffffff DIR=DL STATE=RELEASING EGPRS) free</span><br><span> TBF(DL-TFI_1){RELEASING}: Deallocated</span><br><span style="color: hsl(120, 100%, 40%);">+Testing bts_pch_timer dealloc on bts dealloc</span><br><span style="color: hsl(120, 100%, 40%);">+Starting PCH timer for 2 IMSI</span><br><span style="color: hsl(120, 100%, 40%);">+PCH paging timer started for IMSI=1234</span><br><span style="color: hsl(120, 100%, 40%);">+PCH paging timer started for IMSI=5678</span><br><span style="color: hsl(120, 100%, 40%);">+Deallocating BTS, expecting the PCH timer to be stopped and deallocated</span><br><span style="color: hsl(120, 100%, 40%);">+PCH paging timer stopped for IMSI=1234</span><br><span style="color: hsl(120, 100%, 40%);">+PCH paging timer stopped for IMSI=5678</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-pcu/+/25217">change 25217</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-pcu/+/25217"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-pcu </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ia5e33d1894408e93a51c452002ef2f5758808269 </div>
<div style="display:none"> Gerrit-Change-Number: 25217 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>