| 06:05.49 | *** join/#asterisk-dev infobot (ibot@c-174-52-60-165.hsd1.ut.comcast.net) |
| 06:05.49 | *** topic/#asterisk-dev is Take the March 2019 Asterisk User Survey! https://goo.gl/forms/xL1VUHRsf95saly13 -- Asterisk Development Discussion -=- http://www.asterisk.org/developers -=- Tier 2 and 3.14159265 support is in #asterisk -=- Check out our blog! blogs.asterisk.org -=- Follow on Twitter at @AsteriskDev |
| 07:18.47 | *** join/#asterisk-dev pchero_work (~pchero@87.213.247.82) |
| 08:09.41 | *** join/#asterisk-dev jkroon (~jkroon@41.114.45.77) |
| 08:10.49 | *** join/#asterisk-dev CELYA_ (~Thunderbi@92.184.117.107) |
| 08:27.59 | *** join/#asterisk-dev CELYA_1 (~Thunderbi@92.184.97.13) |
| 08:35.04 | *** join/#asterisk-dev CELYA_ (~Thunderbi@92.184.97.13) |
| 08:41.55 | *** join/#asterisk-dev jkroon (~jkroon@165.16.203.62) |
| 09:12.31 | wdoekes | file: ok, now I'll just straight up quote sean. "i notice you guys are [avoiding] the chan_sip patch" |
| 10:28.41 | *** join/#asterisk-dev GameGamer43 (sid5533@gateway/web/irccloud.com/x-lboyrxduaqtkthev) |
| 10:28.41 | *** mode/#asterisk-dev [+o GameGamer43] by ChanServ |
| 10:46.58 | *** join/#asterisk-dev X-Rob (X-Rob@gateway/web/irccloud.com/x-mzorsbhofbkpdllc) |
| 11:27.02 | *** join/#asterisk-dev jkroon (~jkroon@165.16.203.62) |
| 11:46.19 | *** join/#asterisk-dev moy (sid47040@gateway/web/irccloud.com/x-fpqofxarbantjoos) |
| 11:49.57 | *** join/#asterisk-dev CELYA_ (~Thunderbi@92.184.97.192) |
| 12:33.45 | seanbright | quoting me has never gotten anyone anywhere :P |
| 12:55.42 | wdoekes | perhaps tumbleweeds ð¤ |
| 13:44.20 | file | wobbles |
| 13:44.41 | file | it's on my list, but other stuff has been ahead of it |
| 13:45.03 | file | is responsible for determining what the Asterisk team at Sangoma works on for this year |
| 13:46.53 | *** join/#asterisk-dev sduthil (~sduthil@modemcable094.94-70-69.static.videotron.ca) |
| 14:25.00 | wdoekes | perhaps gtjoseph can take a peek. seeing he did the related options for pjsip |
| 14:28.59 | gtjoseph | eh, chan what? |
| 14:29.07 | gtjoseph | :) |
| 14:41.31 | coreyfarrell | do we have documentation / a guide anywhere on how to determine which res_pjsip* modules actually need to be loaded for an environment? |
| 14:43.08 | file | not really |
| 14:51.59 | *** join/#asterisk-dev bford (uid283514@gateway/web/irccloud.com/x-qfkonextanvkczet) |
| 14:51.59 | *** mode/#asterisk-dev [+o bford] by ChanServ |
| 15:12.36 | *** join/#asterisk-dev CELYA_ (~Thunderbi@92.184.97.128) |
| 15:22.12 | wdoekes | yes. gtjoseph. and nobody uses app_queue nor app_voicemail either :P |
| 15:25.44 | gtjoseph | wdoekes: I'll look at it this morning. |
| 15:26.09 | *** join/#asterisk-dev zerohalo (sid288136@gateway/web/irccloud.com/x-wsiftrkijkcpyvmd) |
| 15:31.58 | *** join/#asterisk-dev kharwell (uid358942@gateway/web/irccloud.com/x-rsiiulhasptackka) |
| 15:31.58 | *** mode/#asterisk-dev [+o kharwell] by ChanServ |
| 15:32.49 | wdoekes | awesome |
| 15:53.16 | gtjoseph | wdoekes: I +1'd it. go ahead and cherry-pick to 16, 17 and master. |
| 16:25.49 | *** join/#asterisk-dev CELYA_1 (~Thunderbi@92.184.100.133) |
| 16:28.12 | wdoekes | nice |
| 17:05.36 | *** join/#asterisk-dev tuxd00d (sid401305@gateway/web/irccloud.com/x-lbcuypheifncmpqd) |
| 18:38.44 | seanbright | can an endpoint accept either sdes or dtls or does it have to be one or the other? |
| 19:06.55 | *** join/#asterisk-dev jkroon (~jkroon@165.16.203.110) |
| 20:01.48 | file | seanbright: I don't remember what it is like these days on incoming... but it used to be one or the other |
| 20:15.34 | seanbright | k |
| 20:16.51 | seanbright | another dumb question: does the testsuite exercise the unit tests in asterisk or do you have to run those manually? |
| 20:17.11 | file | I'll say... manually |
| 20:17.14 | seanbright | k |
| 20:24.18 | kharwell | gtjoseph: regarding the stasis app registry cleanup code. After looking at it, and some of the calls I'm still not entirely sure why it's called at all except when unloading? It seems in every case aside from unload we have access to the app object already and cleanup_cb can be called directly |
| 20:24.46 | gtjoseph | is that good or bad? :) |
| 20:24.47 | kharwell | unless like one comment says it's a lazy way to protect against potential leaks and the list growing too big? |
| 20:24.52 | kharwell | heh not sure really |
| 20:25.16 | gtjoseph | neither am I. that's why I said in the issue that we need to reevaluate the whole thing. |
| 20:25.27 | kharwell | I guess apps can become "stale" and need removal, but we don't have a way to do that so when a new app registers, or unregisters then check all and remove stale ones? |
| 20:26.55 | gtjoseph | well, how do we know it's stale in the first place? |
| 20:27.17 | gtjoseph | whatever action that cause it to go stale should remove it. |
| 20:27.26 | kharwell | yeah |
| 20:27.35 | gtjoseph | rather than having to shotgun the container |
| 20:27.57 | kharwell | agreed. trying to determine that |
| 20:28.13 | kharwell | an app unregisters and deactivates which calls the clean up |
| 20:28.45 | kharwell | but we have the app so could call directly without calling the list |
| 20:28.49 | gtjoseph | the bulk cleanup right? why not just clean up itself? |
| 20:28.56 | kharwell | hrm seems like it might be okay to do that for all cases except unload I"m thinking |
| 20:28.57 | gtjoseph | yeah |
| 20:29.29 | gtjoseph | what criteria does the bulk cleanup use to determine if it should purge an entry or not? |
| 20:31.18 | kharwell | if the app is finished and shutdown |
| 20:31.50 | kharwell | I see now though. If we call it directly then an ao2_unlink will have to be called to make sure it gets removed |
| 20:32.09 | kharwell | as cleanup() is doing that, and using cleanup_cb for the match criteria |
| 20:32.20 | gtjoseph | ask coreyfarrell if weak references can help in this situation. |
| 20:33.43 | kharwell | I don't think we need them. the app object doesn't hold a pointer back to the registry container |
| 20:34.11 | gtjoseph | i'm still not clear on how that callback determines an entry is 'stale" |
| 20:34.25 | gtjoseph | looks |
| 20:34.47 | file | and gtjoseph was never heard from again |
| 20:35.53 | gtjoseph | oh... |
| 20:35.56 | kharwell | I probably shouldn' t have used the word 'stale'. I was thinking based on the code's use of cleanup, and some comments that leaks could occur in the registry. So calling cleanup every once in a while (like on app_register or something) would go over the whole list and remove those apps that are have disconnected (? maybe that's a better word) and should have been removed but for some reason were not |
| 20:36.25 | gtjoseph | yeah so we need to know why they weren't removed in the first place. |
| 20:37.04 | gtjoseph | here's how the callback determines "stale"... |
| 20:37.05 | gtjoseph | ret = app->handler == NULL && ao2_container_count(app->forwards) == 0; |
| 20:37.48 | gtjoseph | does this mean that there's a possibility that unregister was called but there might still be forwards? |
| 20:37.56 | gtjoseph | so we have to check later? |
| 20:38.34 | kharwell | hrm may be |
| 20:39.18 | kharwell | looks like the forwards contain resource subscriptions |
| 20:39.40 | gtjoseph | what do we do with them? |
| 20:40.16 | gtjoseph | and...network subscriptions? or subscriptions to the app? |
| 20:40.40 | gtjoseph | if they're app subs, when we close the app, we should just send them final messages. |
| 20:41.25 | gtjoseph | what a convoluted process :( |
| 20:42.47 | gtjoseph | oh, it's the app's stasis subscriptions to other things |
| 20:42.52 | kharwell | yeah I don't get it? I guess there is a difference between an app shutting down vs deactivating |
| 20:43.38 | gtjoseph | so...when does app->handler go to NULL? that's the other criteria |
| 20:43.49 | kharwell | on deactivation |
| 20:44.11 | kharwell | when it unregisters |
| 20:44.58 | gtjoseph | well, I'd say that when we deactivate we should clean out the subscriptions as well |
| 20:45.10 | file | when does that occur? |
| 20:45.32 | kharwell | it unregisters when the websocket goes down, thus deactivates |
| 20:45.32 | gtjoseph | deacttive only gets called from one place...stasis_app_unregister |
| 20:45.41 | file | so... |
| 20:45.41 | kharwell | but it can be re-activated if the websocket reconnects |
| 20:45.48 | file | don't hit me |
| 20:45.53 | kharwell | so if we dump all subs then when it gets reactivated they are all gone |
| 20:46.01 | file | yeah... |
| 20:46.19 | gtjoseph | well, reestablishing the subs should be part of reactivating |
| 20:46.33 | file | if the websocket drops the existing behavior and what at least one consumer depends on is the world staying the same |
| 20:46.53 | file | did that change? that is: channels remain in the app |
| 20:47.00 | gtjoseph | so in that case, what DOES cleanup the subs? |
| 20:47.05 | file | subscriptions remain, that kinda of thing |
| 20:48.50 | kharwell | explicit unsubscribes |
| 20:48.53 | gtjoseph | well, this is odd... if we call deactivcate on an app, we set the handler to NULL and cleanup the app->data. |
| 20:48.57 | gtjoseph | so.... |
| 20:53.23 | kharwell | seemingly the only way for an app to shutdown is if it is "finished" (i.e. deactivated and no subscriptions). An app deactivates upon websocket disconnect. The only way to remove the subscriptions is through explicit calls. |
| 20:53.31 | gtjoseph | ok so let's think of it another way... do we actually need to lock the container to clean out stale apps? |
| 20:53.56 | gtjoseph | if we miss one, it'll get cleaned up the next time. |
| 20:53.59 | kharwell | So the only way an app is removed from the registry is by unsubbing everything and deactivating it |
| 20:55.30 | kharwell | I guess it's possible for an app to unsubscribe while deactive. So could put a check after every unsubscribe and if deactive and no more subs then remove from registry |
| 20:55.39 | kharwell | meaning, shut down the app |
| 20:56.33 | gtjoseph | yeah but that assumes that if the app got deactivated by a websocket close, someone opens the websocket again. |
| 20:56.40 | kharwell | gtjoseph: not sure. We need to iterate over it. Only a problem I guess if something gets added while going over it? |
| 20:56.45 | gtjoseph | and does the unsubscribes |
| 20:56.52 | kharwell | yeah |
| 20:57.05 | gtjoseph | who cares? if it was added it's hardly likely it'd need to be cleaned up :) |
| 20:57.08 | file | why would they need to open the websocket? |
| 20:57.23 | gtjoseph | well, reestablish a connection to that app. |
| 20:57.32 | file | pour quoi |
| 20:57.33 | kharwell | hrm wait yeah you can post an http command without the websocket listening right |
| 20:57.41 | file | if the app exists, yes |
| 20:57.55 | file | and if channels leave then the subs should disappear through that too |
| 20:58.11 | gtjoseph | yeah sorry. ok |
| 20:58.23 | gtjoseph | so back to my question about not locking the container... |
| 20:58.47 | gtjoseph | most of our issues coming from that locking model |
| 20:58.58 | gtjoseph | so what if we just don't lock the container. |
| 20:59.43 | kharwell | does ao2 handle objects being added to a container during iteration? |
| 20:59.52 | gtjoseph | as a side note, wasn't one of the things from astridevcon to separate websocket from rest? |
| 20:59.56 | kharwell | if you don't lock the container what happens if iterating over it and something gets added/removed? |
| 21:00.03 | gtjoseph | so what? |
| 21:00.09 | file | yes, to sort out connection stuff |
| 21:00.32 | gtjoseph | it's only the cleanup we need to not lock |
| 21:00.46 | gtjoseph | the bulk cleanup that is. |
| 21:02.18 | kharwell | right but if you are cleaning up, and looping over all objects in the container, but while doing so something else is added or removed from the container I'm not sure how ao2 handles that. For instance the iterator might become undefined |
| 21:02.44 | gtjoseph | well, we're not iterating |
| 21:02.49 | gtjoseph | we'tr traversing |
| 21:03.02 | gtjoseph | and traversing doesn't care. |
| 21:03.27 | gtjoseph | i.e. we're running ao2_callback, not ao2_iterate. |
| 21:03.47 | gtjoseph | or container iterate or whatever it's called. |
| 21:03.47 | file | even if you use an ao2_iterator it's fine, it handles that |
| 21:03.51 | kharwell | I can't remember. ao2_callback doesn't use iterators underneath? |
| 21:04.00 | gtjoseph | no |
| 21:04.02 | kharwell | so might be okay then |
| 21:04.15 | gtjoseph | it uses the traverse functions. |
| 21:04.57 | gtjoseph | also, is that container a rw lock container or mutex? |
| 21:05.00 | gtjoseph | i forgot. |
| 21:05.17 | kharwell | mutex |
| 21:05.42 | gtjoseph | maybe it can be RW and then we don't have to lock it for reads. |
| 21:06.04 | gtjoseph | or make sure we're passing the NOLOCK flags correctly |
| 21:06.26 | file | so which are you using, and is the container locked or not? |
| 21:06.32 | kharwell | I think the main problem with the cleanup though causing problems was that it was being called with the registry locked prior. It doesn't need to be, and probably shouldn't. I've moved it out |
| 21:06.48 | kharwell | I also resolved another deadlock between the app object and the session object used in the handler |
| 21:07.37 | gtjoseph | you mean just n ow, or with a previous issue? |
| 21:08.14 | kharwell | just now. In part to fix the issue I was working on. Then I hopefully have fixed the deadlock Ross is running into |
| 21:08.25 | gtjoseph | ah ok. |
| 21:09.01 | gtjoseph | if what you've got now is a reasonable long term fix and we really don't need to do anything else, then I'm fine with that |
| 21:09.29 | gtjoseph | but it's friday afternoon and most of my brain cells are inactive |
| 21:09.58 | kharwell | To fix both issues I don't think I have to "muck" with the cleanup code and locking for that. But altering it as discussed above might help things more long term? stuff that's missed this go around? I'm not sure |
| 21:10.43 | kharwell | yeah I agree. I think I'll let it sit over the weekend and give fresh thought to it on Monday to make sure |
| 21:10.46 | gtjoseph | that's fair. |