Re: [sr-dev] [kamailio/kamailio] async_http_client crash (#2286)

[Daniel-Constantin Mierla]

Closed #2286.

-- 
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/2286#event-3259192174___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

Re: [sr-dev] [kamailio/kamailio] async_http_client crash (#2286)

[Daniel-Constantin Mierla]

OK. Reopen if pops up again.

-- 
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/2286#issuecomment-617599039___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

Re: [sr-dev] [kamailio/kamailio] async_http_client crash (#2286)

[Alex Balashov]

I manually cherry-picked the patch into my clone of 5.3 before it was 
backported, and haven’t had a crash since. 

However, I also hadn’t had a crash the few days prior.

So, it remains to be seen what happens. But I am optimistic this will fix the 
issue. Thank you very much to you both!

—
Sent from mobile, with due apologies for brevity and errors.

> On Apr 21, 2020, at 2:05 PM, Daniel-Constantin Mierla 
>  wrote:
> 
> 
> @abalashov can you try with latest version from stable branch? @grumvalski 
> backported the patch. The avps lists should be no longer there.
> 
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or unsubscribe.
> 
> ___
> Kamailio (SER) - Development Mailing List
> sr-dev@lists.kamailio.org
> https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

Re: [sr-dev] [kamailio/kamailio] async_http_client crash (#2286)

[Daniel-Constantin Mierla]

@abalashov can you try with latest version from stable branch? @grumvalski 
backported the patch. The avps lists should be no longer there.

-- 
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/2286#issuecomment-617323986___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

Re: [sr-dev] [kamailio/kamailio] async_http_client crash (#2286)

[Daniel-Constantin Mierla]

@grumvalski - you can backport to the branches relevant fo the fix. If not, I 
will do it before next releases.

-- 
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/2286#issuecomment-616373328___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

Re: [sr-dev] [kamailio/kamailio] async_http_client crash (#2286)

[Federico Cabiddu]

Sorry the mail was sent before ending by accident.

Thanks for looking into it and fixing it Daniel. For sure it should be
backported.

Cheers,

Federico

On Sat, Apr 18, 2020 at 9:23 AM Federico Cabiddu 
wrote:

> Hi Daniel,
> you are absolutely right and that part of code is for sure wrong. Those
> lines have been there since the beginning and for sure due to my lack of
> understanding of transactions's internals and avps (don't remember why
> sincerely).
> For the records, I've been using mixed suspended/not suspended from the
> beginning but I've always used separate return routes for suspended and not
> suspended queries. That could explain why I never had it.
> Thanks for looking into it
>
> On Sat, Apr 18, 2020 at 3:13 AM Alex Balashov 
> wrote:
>
>> @miconda  To answer a few other questions:
>>
>>1.
>>
>>I always set $http_req(suspend) = 1 on async HTTP client transactions;
>>2.
>>
>>There are no event_routes anywhere;
>>3.
>>
>>The flow is complex, though. After this async HTTP query, there is
>>another *synchronous* HTTP query using the normal http_client module
>>+ mqueue + rtimer -- it was implemented this way because of some
>>problems with transaction-persistent variables (ironically) when 
>> suspending
>>the same TM transaction twice, and a desire for more granular control over
>>the request pipelines given the high and somewhat unpredictable latency of
>>what's being queried.
>>
>> Nevertheless, all this takes place long after the crash point.
>>
>> —
>> You are receiving this because you are subscribed to this thread.
>> Reply to this email directly, view it on GitHub
>> ,
>> or unsubscribe
>> 
>> .
>> ___
>> Kamailio (SER) - Development Mailing List
>> sr-dev@lists.kamailio.org
>> https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
>>
>
___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

Re: [sr-dev] [kamailio/kamailio] async_http_client crash (#2286)

[Federico Cabiddu]

Hi Daniel,
you are absolutely right and that part of code is for sure wrong. Those
lines have been there since the beginning and for sure due to my lack of
understanding of transactions's internals and avps (don't remember why
sincerely).
For the records, I've been using mixed suspended/not suspended from the
beginning but I've always used separate return routes for suspended and not
suspended queries. That could explain why I never had it.
Thanks for looking into it

On Sat, Apr 18, 2020 at 3:13 AM Alex Balashov 
wrote:

> @miconda  To answer a few other questions:
>
>1.
>
>I always set $http_req(suspend) = 1 on async HTTP client transactions;
>2.
>
>There are no event_routes anywhere;
>3.
>
>The flow is complex, though. After this async HTTP query, there is
>another *synchronous* HTTP query using the normal http_client module +
>mqueue + rtimer -- it was implemented this way because of some
>problems with transaction-persistent variables (ironically) when suspending
>the same TM transaction twice, and a desire for more granular control over
>the request pipelines given the high and somewhat unpredictable latency of
>what's being queried.
>
> Nevertheless, all this takes place long after the crash point.
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> ,
> or unsubscribe
> 
> .
> ___
> Kamailio (SER) - Development Mailing List
> sr-dev@lists.kamailio.org
> https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
>
___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

Re: [sr-dev] [kamailio/kamailio] async_http_client crash (#2286)

[Alex Balashov]

@miconda To answer a few other questions:

1. I always set `$http_req(suspend) = 1` on async HTTP client transactions;

2. There are no `event_routes` anywhere;

3. The flow is complex, though. After this async HTTP query, there is another 
_synchronous_ HTTP query using the normal `http_client` module + `mqueue` + 
`rtimer` -- it was implemented this way because of some problems with 
transaction-persistent variables (ironically) when suspending the same TM 
transaction twice, and a desire for more granular control over the request 
pipelines given the high and somewhat unpredictable latency of what's being 
queried.

Nevertheless, all this takes place long after the crash point.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/2286#issuecomment-615530081___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

Re: [sr-dev] [kamailio/kamailio] async_http_client crash (#2286)

[Alex Balashov]

Hi, thank you very much for investigating this!

Unfortunately, the causes of the crash do not appear to arise dependably; as 
luck has it, there hasn't been one in the days since I submitted this issue. 
So, all I can do is cherry-pick this patch into the source and hope for the 
best. 

I will be sure to follow up, but unfortunately there is no control group.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/2286#issuecomment-615529267___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

Re: [sr-dev] [kamailio/kamailio] async_http_client crash (#2286)

[Daniel-Constantin Mierla]

I think I found something that could have been caused the issue, respectively 
setting the AVPs lists from transaction in the process context inside the http 
async callback function. It was done for the case when transaction was 
suspended and no cleaned up, living further in the context of the process and 
when the async callback was executed for a non-suspended-transaction case (with 
faked_msg), then the local avps are cleaned up, but they belonged to a past 
transaction, already destroyed (along with avps).

So this case could happen when there is a mix usage of http async with 
suspended=1 as well as with suspended=0. Code was rather old, but maybe nobody 
used the module mixed suspending modes. 

IMO, there is no need to set avps lists to process local in the process, 
because it is done by faked_env which is used inside t_continue function before 
executing the routing block, so the avps are available inside the config block.

I pushed the commit 1bc3bbd -  @grumvalski can you check it and analyze if 
there are some side effects by removing those lines? I couldn't spot when AVPs 
would be needed before the use of fake_env() by t_continue(), but I am not that 
familiar with http async module internals. If some unwanted effects, then the 
commit can be reverted. Or if all looks ok and tests are fine, then it can be 
backported.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/2286#issuecomment-615366413___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

Re: [sr-dev] [kamailio/kamailio] async_http_client crash (#2286)

[Daniel-Constantin Mierla]

@abalashov have you checked if you set suspend field always? any new crash?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/2286#issuecomment-615355471___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

Re: [sr-dev] [kamailio/kamailio] async_http_client crash (#2286)

[Daniel-Constantin Mierla]

Looking at the backtrace, the execution continues with a `faked_msg` structure:

```
#14 0x0048e205 in run_top_route (a=0x7f3f732aa9c0, msg=0xb04f60 
<_faked_msg>, c=0x0) at core/action.c:1661
#15 0x7f3f705f7d0e in async_http_cb (reply=0x7f3f2b7ee620, 
param=0x7f3f2b7eca18) at async_http.c:256
```

which, based on the code, is done when the query is done without suspending the 
transaction:

  * 
https://github.com/kamailio/kamailio/blob/master/src/modules/http_async_client/async_http.c#L220-L268

Now I see that @abalashov's example is:

```
   $http_req(suspend) = 1;
```

So I wonder if there is an issue propagating it or there are cases in 
@abalashov's config not setting it.

On the other hand, the crash seems to be because in the worker process 
continuing the execution with the faked_msg, the list of avps is not valid, 
maybe they were not reset from a previous execution. The avps there are not for 
sure those you set for the received SIP request, because, again, it is not 
resuming any transaction in this case, so it doesn't get the avps from 
transaction.

@abalashov - do you have event_route blocks in your config? Those are the usual 
suspects running with a faked_msg structure.


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/2286#issuecomment-614556955___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

Re: [sr-dev] [kamailio/kamailio] async_http_client crash (#2286)

[Alex Balashov]

Just had another crash -- identical backtrace; it seems any attempt to access 
this AVP explodes:

```
(gdb) print *avp
Cannot access memory at address 0xabcdefed
```

```
Program terminated with signal 11, Segmentation fault.
#0  0x005d479b in match_by_name (avp=0xabcdefed, id=45, 
name=0x7fff9379c668) at core/usr_avp.c:378
378 if (id==avp->id && avp->flags&AVP_NAME_STR &&
Missing separate debuginfos, use: debuginfo-install 
libuuid-2.23.2-52.el7_5.1.x86_64 nss-pem-1.0.3-4.el7.x86_64
(gdb) set print elements 0
(gdb) where
#0  0x005d479b in match_by_name (avp=0xabcdefed, id=45, 
name=0x7fff9379c668) at core/usr_avp.c:378
#1  0x005d54bf in search_next_avp (s=0x7fff9379c660, 
val=0x7fff9379c630) at core/usr_avp.c:499
#2  0x005d4f04 in search_avp (ident=..., val=0x7fff9379c630, 
state=0x7fff9379c660) at core/usr_avp.c:465
#3  0x005d4913 in search_first_avp (flags=1, name=..., 
val=0x7fff9379c630, s=0x7fff9379c660) at core/usr_avp.c:414
#4  0x7f3f6ba529fd in ops_is_avp_set (msg=0xb04f60 <_faked_msg>, 
ap=0x7f3f73296ed8) at avpops_impl.c:1821
#5  0x7f3f6ba60d47 in w_is_avp_set (msg=0xb04f60 <_faked_msg>, 
param=0x7f3f73296ed8 "", op=0x0) at avpops.c:1059
#6  0x00480f3d in do_action (h=0x7fff9379ce70, a=0x7f3f732a8b30, 
msg=0xb04f60 <_faked_msg>) at core/action.c:1077
#7  0x0048da7c in run_actions (h=0x7fff9379ce70, a=0x7f3f732a8b30, 
msg=0xb04f60 <_faked_msg>) at core/action.c:1576
#8  0x0048e13d in run_actions_safe (h=0x7fff9379dae0, a=0x7f3f732a8b30, 
msg=0xb04f60 <_faked_msg>) at core/action.c:1640
#9  0x00438ff3 in rval_get_int (h=0x7fff9379dae0, msg=0xb04f60 
<_faked_msg>, i=0x7fff9379d318, rv=0x7f3f732a8d70, cache=0x0) at 
core/rvalue.c:915
#10 0x0043d5a3 in rval_expr_eval_int (h=0x7fff9379dae0, msg=0xb04f60 
<_faked_msg>, res=0x7fff9379d318, rve=0x7f3f732a8d68) at core/rvalue.c:1913
#11 0x0043d9f6 in rval_expr_eval_int (h=0x7fff9379dae0, msg=0xb04f60 
<_faked_msg>, res=0x7fff9379d7cc, rve=0x7f3f732a9598) at core/rvalue.c:1921
#12 0x0048097b in do_action (h=0x7fff9379dae0, a=0x7f3f732aa9c0, 
msg=0xb04f60 <_faked_msg>) at core/action.c:1047
#13 0x0048da7c in run_actions (h=0x7fff9379dae0, a=0x7f3f732aa9c0, 
msg=0xb04f60 <_faked_msg>) at core/action.c:1576
#14 0x0048e205 in run_top_route (a=0x7f3f732aa9c0, msg=0xb04f60 
<_faked_msg>, c=0x0) at core/action.c:1661
#15 0x7f3f705f7d0e in async_http_cb (reply=0x7f3f2b7ee620, 
param=0x7f3f2b7eca18) at async_http.c:256
#16 0x7f3f705f0dab in check_multi_info (g=0x7f3f2abeb9e8) at 
http_multi.c:597
#17 0x7f3f705e84ec in event_cb (fd=6, kind=2, userp=0x1d7cdf0) at 
http_multi.c:145
#18 0x7f3f70131a14 in event_process_active_single_queue (activeq=0x1c6f820, 
base=0x1d3ad90) at event.c:1350
#19 event_process_active (base=) at event.c:1420
#20 event_base_loop (base=0x1d3ad90, flags=0) at event.c:1621
#21 0x7f3f705f490f in async_http_run_worker (worker=0x7f3f2abd1bb0) at 
async_http.c:92
#22 0x7f3f705dcb17 in child_init (rank=0) at http_async_client_mod.c:366
#23 0x0057a810 in init_mod_child (m=0x7f3f73245168, rank=0) at 
core/sr_module.c:780
#24 0x0057a4ac in init_mod_child (m=0x7f3f73245d58, rank=0) at 
core/sr_module.c:776
#25 0x0057a4ac in init_mod_child (m=0x7f3f73246340, rank=0) at 
core/sr_module.c:776
#26 0x0057a4ac in init_mod_child (m=0x7f3f732467f8, rank=0) at 
core/sr_module.c:776
#27 0x0057a4ac in init_mod_child (m=0x7f3f73246e38, rank=0) at 
core/sr_module.c:776
#28 0x0057a4ac in init_mod_child (m=0x7f3f73247210, rank=0) at 
core/sr_module.c:776
#29 0x0057a4ac in init_mod_child (m=0x7f3f73247978, rank=0) at 
core/sr_module.c:776
#30 0x0057a4ac in init_mod_child (m=0x7f3f73247da0, rank=0) at 
core/sr_module.c:776
#31 0x0057af5a in init_child (rank=0) at core/sr_module.c:825
#32 0x00426bc1 in main_loop () at main.c:1753
#33 0x0042e63a in main (argc=13, argv=0x7fff9379edb8) at main.c:2802
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/2286#issuecomment-614303159___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

Re: [sr-dev] [kamailio/kamailio] async_http_client crash (#2286)

[Alex Balashov]

Hi -- thanks for the response! 

Alas, nothing has really changed that I know of.

Actually, this is a scenario where multiple HTTP queries are being run as well; 
this is the first stage, but subsequently the scenario is more complicated and 
requires use of the synchronous 'http_client', so it's done manually with 
mqueue/rtimer. I do not remember why simply suspending the transaction twice 
didn't work, I think it had something to do with missing transaction state 
somewhere--I will need to revisit that.

However, the crash occurs long before any of that occurs.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/2286#issuecomment-614285116___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

Re: [sr-dev] [kamailio/kamailio] async_http_client crash (#2286)

[Federico Cabiddu]

Hi Alex,
yes, avp should be persistent. I've never seen a crash like this in 5.2.x (the 
release I'm using in prod) and I'm using avp in resume routes, even 
suspending/resuming twice a single transaction (I have a scenario where two 
http queries are run).
Have anything changed in your system recently, e.g. libcurl version?
I'll try to have a look in the next days to what might have changed in the 
transactions framework that could explain.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/2286#issuecomment-614189317___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev