| 00:19.54 | *** join/#asterisk-dev infobot (ibot@rikers.org) |
| 00:19.54 | *** topic/#asterisk-dev is 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 |
| 05:58.16 | *** join/#asterisk-dev jkroon (~jkroon@uls-154-73-35-201.wall.uls.co.za) |
| 06:38.21 | *** join/#asterisk-dev tsearle (~tsearle@37.19.10.33) |
| 06:38.21 | *** mode/#asterisk-dev [+o tsearle] by ChanServ |
| 07:35.38 | *** join/#asterisk-dev sotoz (~sotoz@095-097-255-066.static.chello.nl) |
| 07:36.16 | *** join/#asterisk-dev hehol (~hehol@gatekeeper.loca.net) |
| 08:07.37 | *** join/#asterisk-dev coreyfarrell (~coreyfarr@24-177-250-191.dhcp.nwtn.ct.charter.com) |
| 08:07.37 | *** mode/#asterisk-dev [+o coreyfarrell] by ChanServ |
| 08:36.34 | *** join/#asterisk-dev jkroon (~jkroon@165.16.204.169) |
| 09:14.43 | jkroon | when receiving a return value from ast_asprintf() (which wraps asprintf()) - should I use ast_free() to free it, or free()? |
| 09:16.41 | coreyfarrell | jkroon: always ast_free unless you are freeing something created directly by an outside library (that case requires work-arounds) |
| 09:19.00 | jkroon | coreyfarrell, the comment for ast_asprintf() states "is a wrapper for asprintf()" implying it's actually calling asprintf in the background, which would use malloc() to allocate the buffer, not ast_malloc ... |
| 09:20.01 | jkroon | never mind, looking at the implementation it's actually using vsnprintf. |
| 09:20.45 | coreyfarrell | jkroon: it uses the internal's of ast_malloc if MALLOC_DEBUG is enabled. |
| 09:21.05 | coreyfarrell | if you use 'free()' on the value allocated by ast_asprintf it will crash when MALLOC_DEBUG is enabled. |
| 09:22.11 | jkroon | yes, that's because ast_asprintf doesn't actually wrap asprintf, it's actually an independent implementation that uses __ast_alloc_region to perform the allocation. |
| 09:26.48 | coreyfarrell | only when MALLOC_DEBUG is enabled. if not then 'free()' of the value allocated by ast_asprintf would not crash, though jenkins would report a compile error for 15+. |
| 10:23.00 | jkroon | uhrm, ok, i think i screwed up on gerrit now, i resubmmitted for ASTERISK-27841 intending to upload patch set 2, however, it came through as patch set 1 on a new change - is there a way to merge/relate them post-publish? |
| 10:23.31 | jkroon | ... looks like I should have manually cloned the Change-Id which I did not ... |
| 10:24.09 | file | Change-Id is how gerrit knows it is the same change, you could have also done "git review -d <review id>" to download and create a local branch, update, and then did git review again |
| 10:24.32 | file | https://wiki.asterisk.org/wiki/display/AST/Gerrit+Usage#GerritUsage-UpdatingaReview |
| 10:25.20 | jkroon | ... feeling stupid now, but that's how one learns. suggested course of action to rectify now? |
| 10:25.32 | jkroon | i'm thinking simply abandon the old change set? |
| 10:25.43 | file | sure |
| 10:26.20 | file | you'll need to fix the new review though, it doesn't build |
| 10:26.48 | jkroon | i did compile test my side, will recheck thanks. |
| 10:28.28 | jkroon | i screwed up the commit :( |
| 10:49.34 | jkroon | is there any way to log in on jenkins to get the full build report? i'd be somewhat interested in looking at that (more out of interest than real need) |
| 10:55.09 | file | same username/password as gerrit |
| 11:12.26 | *** join/#asterisk-dev dan_j (sid21651@gateway/web/irccloud.com/x-gbkcfvjxfgfrrwma) |
| 11:46.35 | jkroon | <PROTECTED> |
| 11:47.27 | coreyfarrell | jkroon: no problem thanks for the fix and quick responses to findings. |
| 11:47.55 | jkroon | i actually enjoy working on asterisk, so call it a diversion :) |
| 11:48.21 | jkroon | wish i had time to be more involved. |
| 12:40.18 | seanbright | i know that bailing out is the correct behavior, but i've always found it funny that in the failure path after dynamic memory allocation fails we allocate memory |
| 12:41.07 | seanbright | what i REALLY wonder is - can an application as complex as asterisk REALLY recover in that scenario |
| 12:41.16 | seanbright | REALLY |
| 12:42.03 | coreyfarrell | not sure but thanks for catching the missing unlock. |
| 12:43.12 | file | seanbright: I wonder the same thing often |
| 12:45.56 | coreyfarrell | I doubt recovering from OOM when swapping is in use is possible. in that case if we actually hit OOM then we're thrashing the swap space so badly that we'd be pretty much locked up. |
| 12:52.15 | seanbright | so we should just change OOM to exit(10) and be done with it |
| 12:52.20 | seanbright | we could eliminate so much code! |
| 12:58.18 | coreyfarrell | sounds controversial but I agree. if Asterisk has leaked and that's why we're OOM then exiting with an error code is for the best - safe_asterisk or systemd can restart us with a clean slate. |
| 13:03.12 | seanbright | i don't see that review getting +1d |
| 13:03.18 | seanbright | just sayin' |
| 13:18.18 | *** join/#asterisk-dev scgm11_ (~scgm11@r186-49-42-113.dialup.adsl.anteldata.net.uy) |
| 13:21.00 | scgm11_ | is there any way to attach connectors to different "sinks" for ARI Statis? for example if I dont want to pass messages to a websocket, if I want any statis event be published somewhere, where do I need to start looking? thanks |
| 13:23.24 | seanbright | i think i speak for all of us when i say... huh? |
| 13:23.39 | file | there's no API or anything for that, res/ari/resource_events.c is the point in between it and Websockets |
| 13:24.13 | seanbright | in the immortal words of socrates... i drank what? |
| 13:28.39 | scgm11_ | thanks |
| 14:13.04 | *** join/#asterisk-dev scgm11_ (~scgm11@r186-49-42-113.dialup.adsl.anteldata.net.uy) |
| 14:15.28 | *** join/#asterisk-dev kharwell (kharwell@nat/digium/x-opaqxrbxkcyjxeay) |
| 14:15.28 | *** mode/#asterisk-dev [+o kharwell] by ChanServ |
| 14:37.50 | *** join/#asterisk-dev bford (uid283514@gateway/web/irccloud.com/x-doreatcpwkbtyqfd) |
| 14:37.50 | *** mode/#asterisk-dev [+o bford] by ChanServ |
| 14:43.33 | *** join/#asterisk-dev scgm11_ (~scgm11@r186-49-42-113.dialup.adsl.anteldata.net.uy) |
| 15:38.23 | *** join/#asterisk-dev cresl1n (Adium@asterisk/libpri-and-libss7-expert/Cresl1n) |
| 15:38.23 | *** mode/#asterisk-dev [+o cresl1n] by ChanServ |
| 15:40.22 | *** join/#asterisk-dev Worldexe (~Worldexe@95-107-33-134.dsl.orel.ru) |
| 15:46.22 | *** join/#asterisk-dev cresl1n (Adium@asterisk/libpri-and-libss7-expert/Cresl1n) |
| 15:46.22 | *** mode/#asterisk-dev [+o cresl1n] by ChanServ |
| 16:32.06 | *** join/#asterisk-dev jkroon (~jkroon@196.33.18.28) |
| 16:35.06 | AsteriskRoss | Before I raise a bug with pjsip, should asterisk send audio to a port in the SDP in a 200 even if the 180 had a different port indicated? I'm seeing audio pass to the 180 port even though the call has been answered. |
| 16:36.21 | file | it should go to the one in the 200 |
| 16:36.58 | AsteriskRoss | Thats what I expected, i'll open a bug |
| 16:43.13 | AsteriskRoss | Ok, this looks like the same as ASTERISK-27441 however this fix didn't get applied to 13 due to the underlying media streams I guessing |
| 16:44.08 | file | streams don't exist in 13, 'nor does that code |
| 16:44.33 | AsteriskRoss | But the same bug though, I'll create a new issue for 13 |
| 16:59.04 | *** join/#asterisk-dev CELYA_ (~Thunderbi@LFbn-1-11898-144.w90-93.abo.wanadoo.fr) |
| 17:20.39 | jkroon | seanbright, good catch @ https://gerrit.asterisk.org/#/c/8938/ is there anything else required from my side? |
| 17:42.06 | coreyfarrell | jkroon: he fixed the issue so I think we're good |
| 18:00.00 | *** join/#asterisk-dev pc-m (~pcm@modemcable094.94-70-69.static.videotron.ca) |
| 18:06.13 | jkroon | he did indeed, i can +1 his patch set ... but i think it's already +2 ... so now it's just wait for it to be merged? |
| 18:11.25 | *** join/#asterisk-dev pc-m (~pcm@modemcable094.94-70-69.static.videotron.ca) |
| 18:21.22 | coreyfarrell | it's gotten two +1's, that not the same as +2 but someone else will do the final approval now. unless you get more feedback (unlikely at this point) you don't need to do anything else. |
| 18:45.25 | jkroon | thank you coreyfarrell , appreciate the feedback (haven't submitted code to asterisk in a very long time) |
| 21:41.31 | coreyfarrell | kharwell: do you have a minute? |
| 21:42.04 | kharwell | yahs |
| 21:42.47 | file | what a trap of a question |
| 21:42.56 | coreyfarrell | I'm a bit unclear about the SO_REUSEADDR thing. wouldn't your patch mean that utils_socket.py would be allowed to bind a TIMED_WAIT port but unless sipp also sets the same option it would fail? |
| 21:42.59 | coreyfarrell | :/ |
| 21:45.15 | kharwell | I don't think that is the case for SO_REUSEADDR, but is for SO_REUSEPORT. Meaning even though SIPp doesn't set it it would be able to use the port bound in the python code. |
| 21:45.45 | kharwell | Now that said yes since SIPp doesn't do that then when we try to bind in python to it it would be shown as in use by the OS |
| 21:45.56 | kharwell | so then the python code would say it is reserved |
| 21:48.33 | kharwell | <PROTECTED> |
| 21:49.49 | kharwell | SIPp then binds to it, but does not set it to be reusable. Next test comes along and if it wants that port and it is in TIME_WAIT then we skip that one and try to locate another, and so forth (at least that is how I believe it should work) |
| 21:51.38 | kharwell | I'm not sure the nuances of SO_REUSEADDR, but maybe if only one side sets it it will allow it no matter what. If that is the case since we mark it as such in python then even if the SIPp one is in TIME_WAIT we can still bind to it and say it is not reserved |
| 21:51.49 | kharwell | So either way it should work. Or at the very least work better than how it has been |
| 21:52.59 | kharwell | Another option is to set the SO_LINGER value to something low and then have the python wait past that, but I wanted to try this option first |
| 21:53.32 | kharwell | (setting SO_LINGER shortens/lengthens the timeout) |
| 21:54.41 | coreyfarrell | ok I think I understand. I'm not sure if you're right but I can't think of a way to test other than brute force. I assume it's not possible to set SO_REUSEADDR after python binds the socket? |
| 21:55.11 | kharwell | not that I am aware. stuff I read said to do it before |
| 22:00.30 | kharwell | I found a good write up about it earlier that laid it out pretty clear. Let me see... |
| 22:01.19 | kharwell | Here it is. Was looking for the differences between SO_REUSEADDR and SO_REUSEPORT: https://stackoverflow.com/questions/14388706/socket-options-so-reuseaddr-and-so-reuseport-how-do-they-differ-do-they-mean-t |
| 22:05.26 | coreyfarrell | kharwell: I'm reading near 'The table above assumes that socketA has already been successfully bound to the address given' |
| 22:07.16 | coreyfarrell | am I correct to look at this like socketA == utils_socket.py, socketB = sipp? |
| 22:07.30 | kharwell | yes |
| 22:08.12 | coreyfarrell | "However, if SO_REUSEADDR is set for the socket you are trying to bind, another socket bound to the same address and port in state TIME_WAIT is simply ignored" |
| 22:10.24 | coreyfarrell | "The code that decides if the bind will succeed or fail only inspects the SO_REUSEADDR flag of the socket fed into the bind() call" <-- this seems to say that if sipp wants to bind a port that is in TIMED_WAIT it would need SO_REUSEADDR? |
| 22:10.57 | kharwell | correct. So socketA (set to reuse and in TIME_WAIT) is what socketB is trying to bind to. So because socketA is set to reuse and is in TIME_WAIT it's current binding/cleanup is ignored when socketB tries to bind to it allowing socketB to do so. |
| 22:13.37 | kharwell | I read that differently. The socket is set to SO_REUSEADDR, which is trying to be bound to. Since it is in TIME_WAIT is it allowed. So socketB can bind to it, but since socketB doesn't set it it gets reinitialized to not be reused and any socket trying to bind after 'b', socketC for instance would fail |
| 22:15.31 | kharwell | hrm just re-read the sentence before that again and it makes me think I read it wrong |
| 22:15.45 | coreyfarrell | to me the last quote I just shared says that bind() looks for SO_REUSEADDR on the current socket, not the port:addr combo. so if the current socket doesn't have SO_REUSEADDR set then it will refuse the socket. |
| 22:16.37 | kharwell | yeah blargh I think you might be correct. If that is the case then SIPp would need to set the socket to SO_REUSEADDR when it tried to bind to it |
| 22:17.40 | coreyfarrell | I think the current change might actually make things worse, it would allow utils_socket.py to take TIMED_WAIT sockets which sipp would be unable to bind. (sorry) |
| 22:19.33 | kharwell | ya could be the case. Right now it's kinda already doing that. Which is what I was attempting to fix. The util_socket binds to the socket to see if it is avail. Then SIPp tries to obtain it, but it's still in TIME_WAIT |
| 22:20.26 | kharwell | hrm I guess I can try lowering the SO_LINGER time and on valid sockets wait that amount of time in utils_socket. thoughts? |
| 22:20.53 | kharwell | could disable it, but according to that write up it is not recommended |
| 22:22.12 | coreyfarrell | weird thing is SO_LINGER should be unneeded for a udp socket. there is no stream (no need for shutdown handshake), and we're not sending any data. |
| 22:22.51 | coreyfarrell | https://stackoverflow.com/questions/32979341/is-it-valid-to-use-so-linger-with-a-udp-socket suggests that SO_LINGER cannot be 0 for udp |
| 22:27.20 | kharwell | we could set it to something low, but after reading the man pages on it it sounds like it will block up to "linger" seconds. That could increase testsuite times by a good bit |
| 22:36.12 | coreyfarrell | I'm playing with a sipp patch right now. I'm no longer convinced this issue can be worked around by python. |
| 22:37.42 | kharwell | yeah you are probably correct. Barring catching the error condition of the process and re-running the test if couldn't bind to port that is. Which would be super gross to do. |
| 22:38.55 | kharwell | with the current utils_socket stuff I think a minor patch to SIPp that just set SO_REUSEADDR before the call to bind would fix things? |
| 22:40.08 | coreyfarrell | http://pastebin.centos.org/746216/ - I haven't even compiled yet but this is what I'm looking at. |
| 22:43.24 | coreyfarrell | the idea is if media_socket_video fails it will close the sockets and go back to the beginning (without resetting the try_counter or media_port) |
| 22:44.19 | kharwell | looks like it should work |
| 22:48.45 | kharwell | it appears that code just keeps trying the same port over again until it is avail or 100 tries is hit? |
| 22:49.02 | kharwell | oh wait nvm I see it being incremented now |
| 22:49.22 | kharwell | wonder in your code if you should inc it by 2 if the video port fails |
| 22:50.30 | coreyfarrell | oh I do need to increment media_port in the video port failure path, otherwise it'll just get the same audio rtp port again and fail on the same video port again. |
| 22:51.58 | coreyfarrell | and I don't need the "Unable to bind video RTP socket" error message, it is unreachable in my patched code. |
| 22:57.10 | kharwell | ya might be easier to just move the video stuff up into the loop and inc media_port by either one or two depending on if audio or video failed. Then the error message after the loop would just apply if could bind to a port for either |
| 23:10.37 | coreyfarrell | kharwell: I just tested what I have, it worked. I'm not sure using a single loop would help things since we'll still need to close the sockets and 'goto rtp_try_again;' on video failure so we can reopen then (since the audio socket bind already succeeded) |
| 23:12.06 | coreyfarrell | for my test I ran python CLI, bound a socket to UDP/6002, then verified tests/channels/SIP/sip_hold failed with the fedora copy of sipp (due to 6002 being blocked), it passed with my copy. |
| 23:12.55 | kharwell | Good deal! Are you planning on submitting that patch or should we attempt to apply it on Jenkins? Or both possibly? |
| 23:15.52 | kharwell | I guess if you think it would take a while to submit it into SIPp I (can't speak for others) wouldn't be opposed to patching the test agents at least. If we go that route I guess create an Asterisk issue with it attached. I'm not sure what all it would take to update SIPp on the agents. I think it downloads the source and then something would have to patch it after? gtjoseph might know |
| 23:16.41 | coreyfarrell | I just posted https://github.com/SIPp/sipp/pull/344 |
| 23:19.35 | coreyfarrell | it looks like wdoekes is a major contributor to sipp, I'm guessing we'll see what he says. he's also the one who posted the issue to github too. |
| 23:20.35 | kharwell | sounds good |
| 23:20.52 | coreyfarrell | I'd say it's up to you guys and gtjoseph if jenkins should get patched now or wait for it to be merged. if we're going to push the patch to jenkins I think it should get an extra set of eyes to make sure we haven't missed something. |
| 23:22.11 | coreyfarrell | oh obviously when I tested the new SIPP I tested it with all 3 utils_socket.py patches reverted |
| 23:22.32 | kharwell | I agree about another set of eyes. I'm getting ready to head out myself, but you might trying pinging more folks tomorrow if they are around |
| 23:22.50 | kharwell | oh yeah those would have to be removed as it would only search once if the media port is specified |
| 23:23.56 | kharwell | or leave the utils_socket code (if it seems like it might be useful for something else), but remove the call to it in the sipp.py file. I guess it would be dead code so prob remove |
| 23:24.52 | coreyfarrell | I'm leaning towards remove (though we'll need to put something into lib/python/asterisk/self_test so the ./self_test doesn't get mad) |
| 23:25.22 | coreyfarrell | or comment out the part of ./self_test that runs tests from that folder. |
| 23:26.27 | kharwell | oh ya forgot about self_test. gotta run. coreyfarrell: thanks for the review, feedback and patching of the code! Cya |
| 23:26.40 | coreyfarrell | have a good night |
| 23:26.57 | kharwell | you too |
| 23:27.06 | *** part/#asterisk-dev kharwell (kharwell@nat/digium/x-opaqxrbxkcyjxeay) |