| 00:04.16 | *** join/#asterisk-dev mjordan (~mjordan@nat/digium/x-psyqeelrxujcsmtr) |
| 00:04.17 | *** mode/#asterisk-dev [+o mjordan] by ChanServ |
| 00:31.49 | *** join/#asterisk-dev w9sh (~sph@50-79-224-193-static.hfc.comcastbusiness.net) |
| 01:07.59 | *** join/#asterisk-dev sgriepentrog (~sgriepent@107-220-72-78.lightspeed.brhmal.sbcglobal.net) |
| 02:24.52 | opticron | fix for codec appending with compatibility restrictions here: https://reviewboard.asterisk.org/r/3763/ |
| 03:01.48 | opticron | and https://reviewboard.asterisk.org/r/3761/ updated for ast_format_none |
| 03:27.52 | *** join/#asterisk-dev mjordan (~mjordan@75.76.55.191) |
| 03:27.52 | *** mode/#asterisk-dev [+o mjordan] by ChanServ |
| 04:04.26 | *** part/#asterisk-dev w9sh (~sph@50-79-224-193-static.hfc.comcastbusiness.net) |
| 04:10.07 | sgriepentrog | mjordan: ping |
| 07:17.18 | *** join/#asterisk-dev tzafrir (~tzafrir@local.xorcom.com) |
| 07:17.18 | *** mode/#asterisk-dev [+o tzafrir] by ChanServ |
| 07:31.49 | *** join/#asterisk-dev sgriepentrog (~sgriepent@107.220.72.78) |
| 10:40.30 | *** join/#asterisk-dev Juggie (~Juggie@unaffiliated/juggie) |
| 10:40.30 | *** join/#asterisk-dev wedhorn (~wedhorn@110-175-181-222.static.tpgi.com.au) |
| 10:40.30 | *** join/#asterisk-dev X-Rob (X-Rob@gateway/web/irccloud.com/x-boeazzlbzlokrgfu) |
| 10:58.37 | *** join/#asterisk-dev Juggie (~Juggie@unaffiliated/juggie) |
| 14:16.54 | *** join/#asterisk-dev [TK]D-Fender (~chatzilla@64.235.216.2) |
| 15:08.21 | mjordan | sgriepentrog: pong (I'll be in & out today) |
| 15:58.50 | opticron | is in as well |
| 16:17.57 | coreyfarrell | opticron: is it your branch I saw that was working on upgrading sched.c/sched.h to operator on ao2 objects? |
| 16:18.27 | opticron | I....think so? a long while back? |
| 16:18.41 | opticron | iirc, I didn't have time to complete things and test |
| 16:19.17 | coreyfarrell | ah.. so no specific technical issue. would you object if I took over where you left off? |
| 16:19.21 | opticron | team/kmoore/scheduler |
| 16:19.27 | opticron | go ahead! |
| 16:20.05 | coreyfarrell | thanks |
| 16:20.10 | opticron | I don't even remember how far I'd gotten |
| 16:20.50 | coreyfarrell | not sure, I just remember seeing the branch after we had some race conditions with scheduler in chan_sip |
| 16:21.06 | opticron | yeah...that was it |
| 16:21.40 | opticron | hasn't had any merge conflicts |
| 16:22.00 | coreyfarrell | oh it's automerging trunk? |
| 16:22.04 | opticron | should be |
| 16:22.11 | opticron | maaaaybe 12 |
| 16:22.41 | coreyfarrell | nope it's trunk |
| 16:22.52 | coreyfarrell | just checked |
| 16:26.12 | opticron | the scheduler gets a bit simpler when everything that it holds on to is ao2 refcounted |
| 16:27.13 | opticron | actually, there's an issue with the todo list as well |
| 16:27.47 | coreyfarrell | todo list? |
| 16:27.54 | opticron | for fixing up the scheduler |
| 16:29.54 | coreyfarrell | is this todo list someplace I can read it? |
| 16:30.08 | opticron | trying to find the issue now |
| 16:32.54 | opticron | ugh...I can't remember enough about the issue to find it :/ |
| 16:35.19 | opticron | I have the early write-up I did as investigation and prelude to actual filing of the issue if nothing else |
| 16:38.18 | opticron | ah-ha, it wasn't an issue, it was a wiki article: https://wiki.asterisk.org/wiki/display/~kmoore/A+Better+Scheduler+API |
| 16:38.30 | opticron | hmmm...you may not be able to see that |
| 16:39.58 | coreyfarrell | page not found :( |
| 16:40.09 | coreyfarrell | after I signed in |
| 16:41.02 | opticron | try now? |
| 16:42.45 | coreyfarrell | nope |
| 16:45.00 | opticron | hmm...shouldn't have any viewing restrictions on that page now |
| 16:46.16 | coreyfarrell | dunno, I even did a Ctrl-F5 and still saying Page not found |
| 16:46.51 | opticron | :/ |
| 16:48.18 | coreyfarrell | I just tried in my second browser, it still asked me to sign in for that page |
| 16:49.07 | opticron | alright, well I can paste the information somewhere else for the time being since the wiki isn't cooperating |
| 16:49.28 | opticron | preference? |
| 16:50.47 | coreyfarrell | I like http://paste.ubuntu.com/ due to no ads |
| 16:51.18 | coreyfarrell | otherwise github might recognize the same wiki tags |
| 16:51.34 | opticron | http://paste.ubuntu.com/7789966/ |
| 16:51.58 | opticron | didn't format too well... |
| 16:52.28 | opticron | that's a bit more work than just changing all users to AO2 |
| 16:52.49 | opticron | but all of them moving to AO2 would be a big step toward that |
| 16:53.53 | coreyfarrell | yeah my main focus is dealing with the object lifetime issues |
| 16:59.30 | coreyfarrell | wow sched.h is heavily used. |
| 16:59.37 | coreyfarrell | will look at this after eating :) |
| 16:59.44 | opticron | yep :) |
| 17:00.49 | coreyfarrell | makes me consider creating a 'sched2' |
| 17:11.52 | *** join/#asterisk-dev seanbrig1t (~sean@asterisk/contributor-and-bug-marshal/seanbright) |
| 17:11.52 | *** mode/#asterisk-dev [+o seanbrig1t] by ChanServ |
| 18:22.39 | *** join/#asterisk-dev tzafrir (~tzafrir@bzq-218-28-58.cablep.bezeqint.net) |
| 18:22.39 | *** mode/#asterisk-dev [+o tzafrir] by ChanServ |
| 18:26.54 | file | yolo |
| 19:38.41 | *** join/#asterisk-dev rmudgett (~rmudgett@nat/digium/x-hcbozgnpktelweta) |
| 19:41.35 | sgriepentrog | So the chan_sip peer leak is due to mwi_event_cb not always calling cleanup on the peer after the last message. I haven't narrowed it down further than that yet, don't have a full grasp of the stasis messaging calls to understand why. |
| 19:50.58 | TazzNZ | hey all |
| 20:10.46 | *** join/#asterisk-dev skrusty (~skrusty@168.63.14.171) |
| 20:20.58 | *** join/#asterisk-dev seanbright (~sean@asterisk/contributor-and-bug-marshal/seanbright) |
| 20:20.58 | *** mode/#asterisk-dev [+o seanbright] by ChanServ |
| 20:21.03 | seanbright | mjordan: you around?/ |
| 20:22.49 | seanbright | any @digium folks around? |
| 20:25.10 | sgriepentrog | i may be the only one... |
| 20:26.11 | seanbright | do you have the ability to bump codec_g729a licenses so that i can reactivate? |
| 20:26.36 | seanbright | reinstalled a machine and i can't re-install the g729 license |
| 20:26.48 | sgriepentrog | Sorry, haven't learned that trick yet. |
| 20:27.04 | seanbright | drat |
| 20:27.21 | seanbright | does support@ work on sundays?/ |
| 20:27.56 | sgriepentrog | Don't know that either. |
| 20:30.25 | seanbright | k. thanks. |
| 20:30.58 | coreyfarrell | sgriepentrog: if you want to email the refs for the leaked object I can take a look |
| 20:35.33 | sgriepentrog | coreyfarrell: done thanks |
| 20:40.50 | sgriepentrog | btw the point at which the ao2_cleanup *should* happen it's the last ref - the chan_sip unload has already been run. |
| 20:41.53 | coreyfarrell | so module_unload is happening before the final mwi_event_cb? |
| 20:42.18 | sgriepentrog | yes exactly. |
| 20:42.20 | coreyfarrell | I'm a bit surprised that doesn't cause a segfault |
| 20:42.50 | sgriepentrog | It's properly held onto the peer ref. Problem is it doesn't always get the last message that causes cleanup. |
| 20:44.07 | sgriepentrog | And I don't understand the stasis messaging enough to know what the contract should be on that "final" message. And I don't see a way around it. |
| 20:44.15 | coreyfarrell | no, I mean if unload_module has already been run then dlclose has been run on chan_sip, so attempting to call mwi_event_cb would segfault |
| 20:45.54 | coreyfarrell | sgriepentrog: what about an unref to peer in destroy_mailbox? |
| 20:46.14 | sgriepentrog | No, I don't think it's that far through the unload sequence. It's started unloading, peers are being destroyed, removed from the peers and peers by ip containers, but after this peer has already been down to a single ref is when the callback happens. |
| 20:46.50 | sgriepentrog | That or I'm not understanding it right. |
| 20:49.34 | coreyfarrell | I'm seeing another leak with this too. if add_peer_mwi_subs is called on the same peer twice.. |
| 20:51.47 | sgriepentrog | that doesn't surprise me. |
| 20:54.25 | coreyfarrell | so when mwi_event_cb has stasis_subscription_final_message what causes the stasis subscription to be removed? |
| 20:56.45 | sgriepentrog | It's already been removed due to peer shutdown. The function where I was trying to unref the peer before unsubscribes - but that was wrong because the peer ref is being held for the final message. |
| 21:01.06 | sgriepentrog | The answer is sip_destroy_peer calls clear_peer_mailboxes calls stasis_unsubscribe. |
| 21:01.34 | sgriepentrog | sorry clear_peer_mailboxes calls destroy_mailbox calls stasis_unsubscribe |
| 21:02.54 | sgriepentrog | Hmm. i may have that wrong. That was when I had the mwi ref +1 disabled to find out what was going on. Sip destroy peer isn't getting called because the ref is held. |
| 21:03.24 | sgriepentrog | But if the final message is received, then it is unref'd, and destroyed, and unsubscribed. |
| 21:04.20 | coreyfarrell | oh fun I think I get it.. because of the circular ref we never get a call to sip_destroy_peer, thus never call stasis_unsubscribe..? |
| 21:05.08 | sgriepentrog | yup. And it's all because the message received by the mwi_callback is an mwi state type and not a final. |
| 21:05.30 | sgriepentrog | I suppose it could be a bug in stasis that the final message isn't getting delivered? |
| 21:05.53 | coreyfarrell | no... it's not being delivered because we never call stasis_unsubscribe |
| 21:05.56 | coreyfarrell | I think |
| 21:06.17 | sgriepentrog | ooh. That could be a thing. |
| 21:06.29 | mjordan | so. I've mucked around in this code in the past. |
| 21:06.37 | sgriepentrog | Hiyo. |
| 21:06.54 | mjordan | A lot of the reason why the peer is ref bumped for the subscription is historical; with the event subsystem, we had a nasty crash where a reload would occur and - during the middle of it - an MWI event would fire |
| 21:07.09 | mjordan | the MWI event would have a stale pointer to the peer, and kerblooey. |
| 21:07.21 | mjordan | Generally, things that have subscriptions to something should not be passing themselves around |
| 21:07.37 | mjordan | Because there is a circular reference here: the destruction of the peer should cause the unsubscription to the MWI topic |
| 21:07.59 | mjordan | But if you've ref bumped the peer for the lifetime of the subscription, you're kind of up a creek |
| 21:08.16 | mjordan | what should probably happen instead is stasis_subscribe should *not* get the peer |
| 21:08.30 | mjordan | instead, it should be the name of the peer |
| 21:08.47 | mjordan | mwi_event_cb should look up the peer, and, if the peer is gone, simply return since the peer is no longer valid |
| 21:08.56 | mjordan | otherwise, the handling should be the same |
| 21:08.59 | sgriepentrog | Hmm. I can try that. |
| 21:09.14 | sgriepentrog | Cool thanks. |
| 21:09.15 | mjordan | circular references suck :-) |
| 21:09.29 | sgriepentrog | yes, and they do so very roundly. |
| 21:09.30 | mjordan | Yeah, I dealt with this crash around 1.8.11 or so. Spent a month trying to figure out why it was happening |
| 21:09.31 | coreyfarrell | no they're great I love them |
| 21:09.53 | mjordan | Plus, the MWI code in chan_sip is prone to deadlocks between the peer and the pvt |
| 21:09.56 | mjordan | yay chan_sip |
| 21:12.04 | coreyfarrell | this is a good example of why I was trying to split chan_sip into files earlier this year.. I stopped because I didn't want to cause a nightmare for file when he tried merging media_formats from 12 to trunk |
| 21:12.21 | mjordan | ha |
| 21:12.23 | mjordan | yeah |
| 21:12.29 | mjordan | coreyfarrell: you're a brave man |
| 21:12.32 | mjordan | many have tried |
| 21:12.34 | mjordan | many have died |
| 21:13.10 | mjordan | You should ask otherwiseguy about that some time. I think he *may* have completely blocked out the memory of that |
| 21:16.32 | coreyfarrell | one thing I'm not in a rush to do is maintain sip.conf compatibility. I think one of the important things that needs to be done with chan_sip is to upgrade to ACO or sorcery, but maintaining compatibility for sip.conf would be difficult |
| 21:18.37 | mjordan | well. ACO would be the closest, but that doesn't help you with dynamic realtime |
| 21:18.59 | mjordan | when otherwiseguy wrote ACO, we kept in mind various use cases in chan_sip. So it can handle things like multiple files (users.conf), and other strangeness. |
| 21:19.16 | mjordan | I'm still not sure it'd be 100% possible, but there's a lot of strange things you can do with that framework to try and accomodate different schemas |
| 21:19.16 | coreyfarrell | oh users.conf |
| 21:19.20 | mjordan | :-) |
| 21:19.24 | mjordan | There's always a wrinkle |
| 21:19.55 | mjordan | sorcery hides parts of ACO, which makes it a bit less flexible - but it needs to do that to get some consistency for the various backends |
| 21:20.29 | mjordan | sorcery would be ideal, but at that point you lose users.conf, and there's a lot of things that would be just flat out broken. |
| 21:20.35 | coreyfarrell | users.conf could be enough of a reason for me to say screw compatibility, and start by copying chan_sip.c to chan_sip2.c |
| 21:20.40 | mjordan | heh |
| 21:21.07 | mjordan | at that point though, why not pjsip? Just the external library? |
| 21:21.07 | coreyfarrell | I'm not joking :) |
| 21:21.41 | coreyfarrell | I might eventually warm up to libpjproject, but it does some things that I'm not a fan of. |
| 21:21.50 | mjordan | memory pools? |
| 21:21.54 | coreyfarrell | thats one |
| 21:22.04 | mjordan | I probably shouldn't point you to my ao2 memory pool branch |
| 21:22.37 | coreyfarrell | asserts that crash asterisk don't give me a warm fuzzy feeling |
| 21:23.12 | coreyfarrell | I use pjproject for a softphone for one of my clients, I've seen a couple of those asserts in it. |
| 21:23.14 | mjordan | well, that could be stopped. |
| 21:23.36 | mjordan | you can compile libpjproject such that its asserts are removed. |
| 21:23.48 | mjordan | which should (IIRC) boil up as failure return calls |
| 21:24.06 | mjordan | coreyfarrell: http://svn.asterisk.org/svn/asterisk/team/mjordan/12-memorypool :-P |
| 21:24.34 | coreyfarrell | does svn co |
| 21:24.36 | mjordan | it actually worked, but performance wasn't that big of an improvement (I was targeting snapshots at the time), so I didn't bother pushing it up |
| 21:24.42 | mjordan | it's out of date |
| 21:24.53 | mjordan | but could be ported forward if we ever wanted a pool of ao2 objects |
| 21:24.57 | mjordan | the pool itself is ref counted |
| 21:25.09 | mjordan | the things inside it 'behave' as if they were ao2 ref counted, but they sit on a throne of lies |
| 21:25.22 | mjordan | when their 'reference' goes to 0, the pool just reclaims them instead of destroying them |
| 21:25.44 | coreyfarrell | that actually wouldn't bother me as much as the pjproject module.. where things are allocated within a pool, and no memory is freed until that pool is freed |
| 21:26.20 | mjordan | I think an ao2 pool would be really great when the pattern of usage is a lot of allocations from a single thread |
| 21:26.23 | coreyfarrell | as for ao2 malloc caching I think the wrinkle is that it needs to zero memory before making it available again |
| 21:26.38 | mjordan | but the multi-threaded aspect of snapshots meant there wasn't much of an improvement. |
| 21:27.11 | mjordan | yeah... I don't think I did that. Basically, there's an init function that can be called, and its up to you if you want to memset it or not |
| 21:28.05 | coreyfarrell | maybe I'm misunderstanding but when i call ao2_alloc(sizeof(*obj), NULL), I expect a pointer to zero'ed memory |
| 21:28.55 | coreyfarrell | so not memset'ing a recycled allocation could not be put into widespread usage |
| 21:29.18 | mjordan | that's true today |
| 21:29.35 | mjordan | the pool thing that I wrote was a bit different. You don't use ao2_alloc, you used a request function on the pool, which gave you back an object |
| 21:30.04 | mjordan | part of the pool creation involved giving it a constructor function that would be called on the object when you requested it |
| 21:30.18 | mjordan | http://svn.asterisk.org/svn/asterisk/team/mjordan/12-memorypool/tests/test_astobj2_memory_pool.c |
| 21:31.22 | mjordan | believe me, it wasn't ready for peer review |
| 21:32.03 | mjordan | hence why it sits languishing on a branch. And I'm not sure it's useful anyway; I wouldn't try it out on something unless performance analysis showed us spending a lot of time in malloc |
| 21:34.29 | coreyfarrell | the thing that really made me dislike pjproject's memory pools was when I looked at the code that manipulates pj_str |
| 22:53.39 | *** join/#asterisk-dev coreyfarrell (~cfarrell@2001:4830:117f:2:e4ee:cf3:ebac:9e94) |
| 23:11.32 | *** join/#asterisk-dev luckman212 (~luckman21@unaffiliated/luckman212) |
| 23:20.57 | *** join/#asterisk-dev ipengineer (~zconkle@108-206-120-218.lightspeed.mckntx.sbcglobal.net) |
| 23:38.12 | *** join/#asterisk-dev snuff-work (~snuffy@210.8.167.13) |
| 23:38.13 | *** mode/#asterisk-dev [+o snuff-work] by ChanServ |