| 02:30.45 | *** join/#asterisk-dev luckman212 (~luckman21@unaffiliated/luckman212) |
| 03:35.35 | *** join/#asterisk-dev nix8n82 (~luser@67-130-74-235.dia.static.qwest.net) |
| 03:40.50 | *** join/#asterisk-dev snuff-work (~blatus@203.87.61.22) |
| 03:40.50 | *** mode/#asterisk-dev [+o snuff-work] by ChanServ |
| 06:29.47 | *** join/#asterisk-dev goddva (~goddva@77.40.154.242) |
| 07:18.52 | *** join/#asterisk-dev bof22 (~Thunderbi@185.13.183.107) |
| 08:10.56 | *** join/#asterisk-dev hehol (~hehol@gatekeeper.loca.net) |
| 08:11.11 | *** join/#asterisk-dev oej (~oej@h87-96-134-129.dynamic.se.alltele.net) |
| 08:11.12 | *** mode/#asterisk-dev [+o oej] by ChanServ |
| 08:21.29 | *** join/#asterisk-dev tzafrir (~tzafrir@local.xorcom.com) |
| 08:21.30 | *** mode/#asterisk-dev [+o tzafrir] by ChanServ |
| 09:15.44 | *** join/#asterisk-dev bof22 (~Thunderbi@185.13.183.107) |
| 12:10.20 | *** join/#asterisk-dev bof22 (~Thunderbi@185.13.183.107) |
| 13:09.12 | *** join/#asterisk-dev matesstar (d4478a32@gateway/web/freenode/ip.212.71.138.50) |
| 14:20.02 | *** join/#asterisk-dev gtjoseph (~gtj0@unaffiliated/gtj) |
| 15:00.41 | *** join/#asterisk-dev malcolmd (malcolmd@pdpc/sponsor/digium/malcolmd) |
| 15:00.41 | *** mode/#asterisk-dev [+o malcolmd] by ChanServ |
| 15:12.13 | *** join/#asterisk-dev kharwell (kharwell@nat/digium/x-djmucqoukktlwvuv) |
| 15:24.38 | *** join/#asterisk-dev [TK]D-Fender (~chatzilla@216-191-106-163.dedicated.allstream.net) |
| 15:37.59 | *** join/#asterisk-dev newtonr (RustyNewto@nat/digium/x-piqtdyuxxtmfkuah) |
| 15:37.59 | *** mode/#asterisk-dev [+o newtonr] by ChanServ |
| 15:43.57 | *** join/#asterisk-dev zerohalo (~zerohalo@2601:199:4200:d92e:1dea:3591:746:bcd3) |
| 16:21.27 | *** join/#asterisk-dev bof22 (~Thunderbi@185.13.183.107) |
| 16:28.43 | *** join/#asterisk-dev rmudgett (rmudgett@nat/digium/x-zsowpmwloaoswzvz) |
| 16:42.15 | *** join/#asterisk-dev luckman212 (~luckman21@unaffiliated/luckman212) |
| 17:14.01 | *** join/#asterisk-dev goddva (~goddva@cm-84.209.8.215.getinternet.no) |
| 19:46.35 | *** join/#asterisk-dev goddva (~goddva@2a02:fe0:c511:3800:ae9e:17ff:fe91:a8c9) |
| 19:55.27 | *** join/#asterisk-dev tzafrir (~tzafrir@bzq-179-40-172.cust.bezeqint.net) |
| 19:55.27 | *** mode/#asterisk-dev [+o tzafrir] by ChanServ |
| 21:24.59 | *** join/#asterisk-dev leedm777 (leedm777@nat/digium/x-kdccewkznbvxicfc) |
| 22:35.39 | kharwell | when adding extensions (pbx core) using is there any reason that the contexts should not be locked? Looks like there might be a path or two that adds an extension without locking it |
| 22:42.02 | file | kharwell, I think it... depends. Contexts can exist outside of the PBX core, essentially, until merged. |
| 22:44.55 | kharwell | I was looking at ast_add_extensions2 function. It looks like from within pbx core it is called with the contexts locked. Outside as you noted is a different matter. Would it hurt to put a lock/unlock in ast_add_extensions2? |
| 22:46.56 | kharwell | Being recursive if it was already locked it should be okay. If it wasn't then it will lock it before calling ast_add_extension2_lockopt which should guarantee the contexts to always be locked once in ast_change_hint |
| 22:47.56 | kharwell | then it could be unlocked before locking the context_merge_lock |
| 22:48.21 | file | maybe? :D |
| 22:48.57 | kharwell | deletes pbx.c |
| 22:49.18 | file | it sounds like an idea |
| 22:52.17 | coreyfarrell | kharwell: at one point stuff in pbx.c used rwlocks, I think some of the non-locking functions are left-over from when recursive locking was a problem |
| 22:52.46 | file | it's a mix of both |
| 22:52.54 | coreyfarrell | but I'll also +1 `git rm main/pbx.c` |
| 22:53.43 | file | it could be a fun challenge to replace it, but in reality... testing is hellish |
| 22:55.11 | coreyfarrell | pbx.c needs to be split into multiple sources |
| 22:56.19 | gtjoseph | Sounds like Christmas holiday project. :) |
| 22:56.36 | coreyfarrell | gtjoseph, I'll be watching for your review request |
| 22:56.53 | gtjoseph | I didn't say which Christmas. |
| 23:01.07 | gtjoseph | actually, I wonder if it would be that hard just to take all those little utilities and put them into a separate file. |
| 23:02.25 | gtjoseph | then maybe the pbx_builtins into another |
| 23:30.57 | kharwell | actually now I am wondering the opposite. I get having to lock the contexts when accessing the lists of contexts, but seems once you have the context that one is adding an extension to you wouldn't need a lock on contexts. The context itself would need locking, but not contexts list/table |
| 23:42.03 | coreyfarrell | kharwell, doesn't the contexts lock protect against another thread running ast_merge_contexts_and_delete and destroying the context after making a copy of it? |
| 23:44.08 | kharwell | coreyfarrell: hmm maybe. That is something I will need to look into |
| 23:48.30 | coreyfarrell | kharwell: pretty sure the call to __ast_internal_context_destroy in a loop inside ast_merge_contexts_and_delete is what you need to protect against, any (struct ast_context*) is only valid during while the contexts are locked |
| 23:51.14 | kharwell | so if I understand then what that means is the context that is having the extension added to could just go away while adding the extension? |
| 23:53.38 | coreyfarrell | I think all context objects are freed during a ast_merge_contexts_and_delete.. called by reload of pbx_ael, pbx_config or pbx_lua |
| 23:55.09 | kharwell | that looks to be the case |