### Description
Setting string values via `cfg_rpc` leaves old strings in SHM.
#### Reproduction
<!-- If the issue can be reproduced, describe how it can be done. --> Set some string value to e.g. `tm.ac_extra_hdrs`: ```shell x=10 while [ $x -gt 0 ] ; do sudo kamctl rpc cfg.sets tm ac_extra_hdrs das$x >/dev/null sleep 0.001 x=$((x - 1)) done ```
#### Debugging Data
After running above snippet four times: ``` sudo kamcmd cfg.set_now_int core mem_dump_shm 1 grep "^Oct 4 15:11:4.* qm_status:.*alloc'd" /var/log/syslog | cut -d: -f6- | sort | uniq -c | sort -n | grep core/cfg/cfg 1 alloc'd from core: core/cfg/cfg_struct.c: cfg_shmize(217) 1 alloc'd from core: core/cfg/cfg_struct.c: sr_cfg_init(322) 1 alloc'd from core: core/cfg/cfg_struct.c: sr_cfg_init(334) 1 alloc'd from core: core/cfg/cfg_struct.c: sr_cfg_init(346) 1 alloc'd from core: core/cfg/cfg_struct.c: sr_cfg_init(353) 1 alloc'd from core: core/cfg/cfg_struct.c: sr_cfg_init(360) 2 alloc'd from core: core/cfg/cfg_ctx.c: cfg_register_ctx(47) 3 alloc'd from core: core/cfg/cfg_struct.c: cfg_clone_global(625) 39 alloc'd from core: core/cfg/cfg_ctx.c: cfg_set_now(559) 40 alloc'd from core: core/cfg/cfg_struct.c: cfg_child_cb_new(828) 42 alloc'd from core: core/cfg/cfg_struct.c: cfg_clone_str(130) sudo kamcmd mod.stats core shm|grep \bcfg_ cfg_clone_global(625): 1392 cfg_set_now(559): 624 cfg_clone_str(130): 384 cfg_shmize(217): 696 cfg_register_ctx(47): 64 cfg_child_cb_new(828): 2560 ``` memory values from cfg_clone_str addresses: ``` grep -B1 'Oct 4 15:11:4.*cfg_clone_str' /var/log/syslog|sed '/address=/!d' | cut -d: -f6- 55. N address=0x7f1310eab7f8 frag=0x7f1310eab7c0 size=40 used=1 56. N address=0x7f1310eab888 frag=0x7f1310eab850 size=24 used=1 62. N address=0x7f1310ec7b10 frag=0x7f1310ec7ad8 size=8 used=1 65. N address=0x7f1310ec7ca0 frag=0x7f1310ec7c68 size=8 used=1 68. N address=0x7f1310ec7e30 frag=0x7f1310ec7df8 size=8 used=1 69. N address=0x7f1310ec7ea0 frag=0x7f1310ec7e68 size=8 used=1 72. N address=0x7f1310ec8030 frag=0x7f1310ec7ff8 size=8 used=1 75. N address=0x7f1310ec81c0 frag=0x7f1310ec8188 size=8 used=1 78. N address=0x7f1310ec8350 frag=0x7f1310ec8318 size=8 used=1 81. N address=0x7f1310ec84e0 frag=0x7f1310ec84a8 size=8 used=1 84. N address=0x7f1310ec8670 frag=0x7f1310ec8638 size=8 used=1 87. N address=0x7f1310ec8800 frag=0x7f1310ec87c8 size=8 used=1 90. N address=0x7f1310ec8990 frag=0x7f1310ec8958 size=8 used=1 93. N address=0x7f1310ec8b20 frag=0x7f1310ec8ae8 size=8 used=1 96. N address=0x7f1310ec8cb0 frag=0x7f1310ec8c78 size=8 used=1 99. N address=0x7f1310ec8e40 frag=0x7f1310ec8e08 size=8 used=1 103. N address=0x7f1310ec92f0 frag=0x7f1310ec92b8 size=8 used=1 106. N address=0x7f1310ec9480 frag=0x7f1310ec9448 size=8 used=1 109. N address=0x7f1310ec9610 frag=0x7f1310ec95d8 size=8 used=1 112. N address=0x7f1310ec97a0 frag=0x7f1310ec9768 size=8 used=1 115. N address=0x7f1310ec9930 frag=0x7f1310ec98f8 size=8 used=1 118. N address=0x7f1310ec9ac0 frag=0x7f1310ec9a88 size=8 used=1 121. N address=0x7f1310ec9c50 frag=0x7f1310ec9c18 size=8 used=1 124. N address=0x7f1310ec9de0 frag=0x7f1310ec9da8 size=8 used=1 127. N address=0x7f1310ec9f70 frag=0x7f1310ec9f38 size=8 used=1 131. N address=0x7f1310eca420 frag=0x7f1310eca3e8 size=8 used=1 134. N address=0x7f1310eca5b0 frag=0x7f1310eca578 size=8 used=1 137. N address=0x7f1310eca740 frag=0x7f1310eca708 size=8 used=1 140. N address=0x7f1310eca8d0 frag=0x7f1310eca898 size=8 used=1 144. N address=0x7f1310ecad80 frag=0x7f1310ecad48 size=8 used=1 147. N address=0x7f1310ecaf10 frag=0x7f1310ecaed8 size=8 used=1 150. N address=0x7f1310ecb0a0 frag=0x7f1310ecb068 size=8 used=1 153. N address=0x7f1310ecb230 frag=0x7f1310ecb1f8 size=8 used=1 156. N address=0x7f1310ecb3c0 frag=0x7f1310ecb388 size=8 used=1 159. N address=0x7f1310ecb550 frag=0x7f1310ecb518 size=8 used=1 162. N address=0x7f1310ecb6e0 frag=0x7f1310ecb6a8 size=8 used=1 165. N address=0x7f1310ecb870 frag=0x7f1310ecb838 size=8 used=1 168. N address=0x7f1310ecba00 frag=0x7f1310ecb9c8 size=8 used=1 172. N address=0x7f1310ecbeb0 frag=0x7f1310ecbe78 size=8 used=1 175. N address=0x7f1310ecc040 frag=0x7f1310ecc008 size=8 used=1 178. N address=0x7f1310ecc1d0 frag=0x7f1310ecc198 size=8 used=1 181. N address=0x7f1310ecc360 frag=0x7f1310ecc328 size=8 used=1 0x7f1310eab7f8: "trying -- your call is important to us" 0x7f1310eab888: "Server Internal Error" 0x7f1310ec7b10: "das10" 0x7f1310ec7ca0: "das9" 0x7f1310ec7e30: "das10" 0x7f1310ec7ea0: "das10" 0x7f1310ec8030: "das9" 0x7f1310ec81c0: "das9" 0x7f1310ec8350: "das8" 0x7f1310ec84e0: "das7" 0x7f1310ec8670: "das6" 0x7f1310ec8800: "das5" 0x7f1310ec8990: "das4" 0x7f1310ec8b20: "das3" 0x7f1310ec8cb0: "das2" 0x7f1310ec8e40: "das1" 0x7f1310ec92f0: "das10" 0x7f1310ec9480: "das9" 0x7f1310ec9610: "das8" 0x7f1310ec97a0: "das7" 0x7f1310ec9930: "das6" 0x7f1310ec9ac0: "das5" 0x7f1310ec9c50: "das4" 0x7f1310ec9de0: "das3" 0x7f1310ec9f70: "das2" 0x7f1310eca420: "das1" 0x7f1310eca5b0: "das8" 0x7f1310eca740: "das7" 0x7f1310eca8d0: "das6" 0x7f1310ecad80: "das5" 0x7f1310ecaf10: "das4" 0x7f1310ecb0a0: "das3" 0x7f1310ecb230: "das2" 0x7f1310ecb3c0: "das1" 0x7f1310ecb550: "das8" 0x7f1310ecb6e0: "das7" 0x7f1310ecb870: "das6" 0x7f1310ecba00: "das5" 0x7f1310ecbeb0: "das4" 0x7f1310ecc040: "das3" 0x7f1310ecc1d0: "das2" 0x7f1310ecc360: "das1" ```
### Additional Information
* **Kamailio Version** - output of `kamailio -v`
``` /usr/sbin/kamailio -v version: kamailio 5.2.4 (x86_64/linux) flags: STATS: Off, USE_TCP, USE_TLS, USE_SCTP, TLS_HOOKS, USE_RAW_SOCKS, DISABLE_NAGLE, USE_MCAST, DNS_IP_HACK, SHM_MEM, SHM_MMAP, PKG_MALLOC, Q_MALLOC, F_MALLOC, TLSF_MALLOC, DBG_SR_MEMORY, USE_FUTEX, FAST_LOCK-ADAPTIVE_WAIT, USE_DNS_CACHE, USE_DNS_FAILOVER, USE_NAPTR, USE_DST_BLACKLIST, HAVE_RESOLV_RES ADAPTIVE_WAIT_LOOPS=1024, MAX_RECV_BUFFER_SIZE 262144 MAX_URI_SIZE 1024, BUF_SIZE 65535, DEFAULT PKG_SIZE 8MB poll method support: poll, epoll_lt, epoll_et, sigio_rt, select. id: unknown compiled with gcc 6.3.0 ```
* **Operating System**:
``` Debian 9.11 ```
I looked today into it as well. I could reproduce it with current git master.
``` root@proxy-1:/usr/local/src/kamailio-devel# grep "^Oct 7 18:38.* qm_status:.*alloc'd" /var/log/syslog | cut -d: -f6- | sort | uniq -c | sort -n | grep core/cfg/cfg 1 alloc'd from core: core/cfg/cfg_struct.c: cfg_shmize(217) 1 alloc'd from core: core/cfg/cfg_struct.c: sr_cfg_init(324) 1 alloc'd from core: core/cfg/cfg_struct.c: sr_cfg_init(336) 1 alloc'd from core: core/cfg/cfg_struct.c: sr_cfg_init(348) 1 alloc'd from core: core/cfg/cfg_struct.c: sr_cfg_init(355) 1 alloc'd from core: core/cfg/cfg_struct.c: sr_cfg_init(362) 2 alloc'd from core: core/cfg/cfg_ctx.c: cfg_register_ctx(47) 2 alloc'd from core: core/cfg/cfg_struct.c: cfg_clone_global(627) 80 alloc'd from core: core/cfg/cfg_ctx.c: cfg_set_now(559) 81 alloc'd from core: core/cfg/cfg_struct.c: cfg_child_cb_new(830) 84 alloc'd from core: core/cfg/cfg_struct.c: cfg_clone_str(130) root@proxy-1:/usr/local/src/kamailio-devel# kamcmd core.version kamailio 5.4.0-dev0 (x86_64/linux) 7cd263-dirty ```
I haven't really went that deep in the related code, but there are reference counters behind those values, so they might be released when all processes get to use the new value, which can be later in time (or never, depending on the type of sip traffic). Overall, I am not sure if the framework was designed to deal with very often changes, could be more for adjusting some global/custom/module parameters from time to time to avoid restarting kamailio.
With the first example in this report, changing very often the ac_extra_hdrs for tm is not likely, imo.
If you need to use in config a string variable that has to be changed very often via rpc, use better $shv(x) or $sht(t=>x) .
I tested it with another cfg variable not related to any module (like we have pstn.gw_ip in default cfg). The test system did not have any traffic, so i would assume that the variable would be commited really fast. But I will have another look to it.
That could actually be part of this case -- the values are cloned by each process to avoid sync'ing with mutexes at runtime. Each process gets the new value when it executes cfg_update(), which is typically done when a process has something new to do (e.g., process a new sip message). In your test case, as the old value is still referenced by other processes, might not be destroyed.
This part of code is inherited from SER project, I am not very familiar with, that's why I am saying it might be designed for a different purpose than is expected here (i.e., not for use with very often changes of values). My comments here are based on a brief look at the code.
And, as I said, for needs related to values that need to be changed very often via rpc, better use shared variable or shared hash table variables.
Yes, changing a cfg value e.g. every minute with this framework makes no sense. But good comment about the update mechanism, I will play with it e.g. by sending some SIP messages. I also looked into the code - but I am also not deeply familiar with the code, only looked to it as well.
Daniel is right, the memory should be eventually freed when ALL the child processes update their local configuration, i.e. do at least one iteration of their loop and call cfg_update(). Each child process updates its local cfg pointer to the global one and travels through the pending callback list. Every item of this list can have a set of memory address to free. The last child that processes the change frees the memory. Check cfg_update_local with the loop: while (cfg_child_cb != last_cb) {...} cfg_child_cb_free_item() frees the "replaced" strings.
The restriction is that none of the processes should be idle for a very long time. They should at least update the local config from time to time. This may not be always the case.
Thanks for the additional insight! So did I understood you correctly: If I use Kamailio in a configuration with several children listening on different network interfaces - if there is at least one child that does not handle traffic the "replaced" strings are not freed?
Yes, this happens. The function updating the config block and replacing some strings of that with newly allocated values cannot free the old ones because it does not know if a child might still use the old values. We have to wait for all the children to release the values. One approach could be to wake up the children from time to time with a signal, even if there is no traffic.
Thank you for the feedback. Ok, this would be a bit more complex change. I will need to get some time to look into it.
This issue is stale because it has been open 6 weeks with no activity. Remove stale label or comment or this will be closed in 2 weeks.
We will investigate how complex it is to change/fix.
This issue is stale because it has been open 6 weeks with no activity. Remove stale label or comment or this will be closed in 2 weeks.
Closed #2094 as not planned.
-
Bastian Triller -
Daniel-Constantin Mierla
-
github-actions[bot]
-
Henning Westerholt
-
mtirpak