[Home]

Summary:ASTERISK-00465: reload from "remote console" will crash asterisk
Reporter:Paul Cadach (pcadach)Labels:
Date Opened:2003-10-30 13:18:17.000-0600Date Closed:2011-06-07 14:00:51
Priority:CriticalRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) h323-cvs.diff
( 1) h323-cvs2.diff
Description:Connect to asterisk with 'asterisk -r' then do 'reload' will crash *. But if you first do 'unload chan_h323.so' then 'load chan_h323.so', reload will NOT crash *. Reload at "real" * console will not crash * too (independedly on re-loading chan_h323.so module).

****** ADDITIONAL INFORMATION ******

My "reload" patch must be applied to make monitor thread stop before unloading.
Comments:By: x martinp (martinp) 2003-11-03 18:11:36.000-0600

You propably use the old code of h323. The problem is that even if you issue reload command in the main console it'll crash if you use gatekeeper. By the way do you use gatekeeper ?
By: Paul Cadach (pcadach) 2003-11-03 20:56:46.000-0600

Yep, I uses gatekeeper and will try to play with chan_h323 from latest cvs (but I have about a patch sized about 11K for chan_h323 from 0.5.0 which I uses now).
By: jerjer (jerjer) 2003-11-04 00:42:56.000-0600

Patch?

use cvs code
By: Paul Cadach (pcadach) 2003-11-04 01:17:19.000-0600

I see nothing new in the cvs from 0.5.0 except for:
channels/h323/ast_h323.cpp:
revision 1.25
date: 2003/09/22 06:21:03;  author: jeremy;  state: Exp;  lines: +2 -47
rollback transfer support...not properly implemented
----------------------------
channels/h323/ast_h323.h:
revision 1.11
date: 2003/09/22 23:19:27;  author: jeremy;  state: Exp;  lines: +2 -3
oopsie remove it from here too
----------------------------
channels/chan_h323.c:
revision 1.9
date: 2003/10/21 08:52:51;  author: jeremy;  state: Exp;  lines: +2 -2
make chan_h323 not load if no config file is found
----------------------------

I don't think removed transfer support was cause of crash at reload when chan_h323 uses gatekeeper... Also, why this crash don't reproducible after chan_h323.so reloading (by unload and next load from CLI) and by reload from real console (not asterisk -r)?

As I can see, unload_module() in chan_h323.c don't stops monitoring thread which was fixed in my unload patches (fixes was for proper ast_cli_unregister() calls in other channels and applications too).

Also, I think pthread_testcancel() in do_monitor() must be called AFTER ast_io_wait() call because monitor termination sequence will kill thread with SIGURG which will stops ast_io_wait() and then thread cancellation condition must be checked immediately.

edited on: 11-04-03 01:20
By: jerjer (jerjer) 2003-11-05 00:45:02.000-0600

what is this patch you talk about?
By: Paul Cadach (pcadach) 2003-11-05 01:12:19.000-0600

I mean my ticket ASTERISK-454 with collection of patches to make asterisk to be more clean on stop/reload.

edited on: 11-05-03 01:08
By: Brian West (bkw918) 2003-11-05 01:35:30.000-0600

I never have * crash on reload with chan_h323

bkw
By: Paul Cadach (pcadach) 2003-11-05 01:53:55.000-0600

Can you report your versions of:
- openh323;
- pwlib;
- gcc used to compile *;
- glibc;
- kernel?
I have 2 systems runs asterisk - one is RH-7.0 with updated gcc (2.96-112) and one is RH-7.3 with glibc from RH-9.0 (glibc-2.3.2). At first listed system h.323 show codecs works fine but second crashes (second also have * malloc debugging compiled in):
*CLI> h.323 show codecs
 0:04.254      H.323 Chan...r Asterisk       assert.cxx(105)   PWLib   Assertion fail: Null pointer reference, file /home/openh323/pwlib/include/ptlib/contain.inl, line 203

<A>bort, <C>ore dump, <I>gnore?
By: Paul Cadach (pcadach) 2003-11-05 14:15:19.000-0600

Reminder sent to JerJer

Jeremy, could you tell me your e-mail address (my is paul@odt.east.telecom.kz) - I'll send you my "cumulative" patch to chan_h323 with comments?
By: jerjer (jerjer) 2003-11-09 12:04:22.000-0600

submit the diff here...make sure its against the latest cvs
By: Paul Cadach (pcadach) 2003-11-09 13:40:00.000-0600

I'll test and submit patch with comments tomorrow? But it's not too clean - at least some places needs to be checked (but it works for me fine). Short list of changes (without detailed comments why its made) is:
1) add procedure to format aliases to strip OpenH323-added comments like "<main alias> (<other aliases>) [<ip address>]" (replaced everythere where aliases is required);
2) "manually" include/exclude endpoint's options to switch from endpoint to gateway (for me OpenH323 have "local" terminalType variable which used in H323EndPoint::SetEndpointTypeInfo() instead of terminalType from MyH323EndPoint class);
3) setup Caller-ID to be more comfortable (split user name and phone to allow OpenH323 to set Q931's IE:DisplayName and IE:DisplayNumber correctly);
4) logical channel startup sets up RTP stream more correctly (directly with negotiated payload format, really channel->nativeformats needs to be individual for each direction);
5) add input indications capabilities to negotiation process;
6) allow RTCP packets to be readed-out by ast_rtcp_read() (huh! RTCP fully ignored by *'s RTP stack for now);
7) setup translations and translate "on-the-fly" invalid packets passed to oh323_write() (smoother issue? - 1-2 packets gets in old format after changing native format and translations are set up);
8) terminate monitor thread AFTER waiting for I/O instead of BEFORE.
By: Paul Cadach (pcadach) 2003-11-10 12:19:14.000-0600

Ok, this is a patch. It must be cleanly apply to the current CVS tree. List of changes was in previous note, comments are inluded "inline" in the patch as comments in source code.

Waiting for feedback from you, Jeremy...
By: jerjer (jerjer) 2003-11-24 11:44:47.000-0600

I need someone else voutch for this patch. I no longer have any H.323 gear to test with and I'm not about to commit this size of a patch without an independant test for sanity.
By: jerjer (jerjer) 2003-12-08 23:13:36.000-0600

still waiting for someone else to voutch for this massive patch
By: Paul Cadach (pcadach) 2003-12-21 02:13:42.000-0600

Attached patch (cvs2.diff) is based on CVS dated by Dec, 15 2003.
By: jerjer (jerjer) 2003-12-23 16:44:22.000-0600

I applied this patch, with changes to cvs.  I need someone to test this.
By: jerjer (jerjer) 2003-12-23 17:03:17.000-0600

This massive patch does not fix the -r reload problem.  Misrepresnation
By: jerjer (jerjer) 2003-12-23 21:23:32.000-0600

This patch is totally broken. Please test your code before building a diff.  There is no way this code actually worked on your system
By: jerjer (jerjer) 2003-12-23 21:43:27.000-0600

Reminder sent to PCadach

Now you get to fix the cvs head of chan_h323.
By: Paul Cadach (pcadach) 2003-12-23 21:44:33.000-0600

Jeremy, you asked me at November, 11th to post my cumulative patch here, so I did it. Bug noticed in this ticked is still exists - I don't know how to fix it for now (but still trying to figure out). Cumulative patch adds some features/fixes for H.323 driver (like but reload from 'asterisk -r' is still an issue to fix.
By: Paul Cadach (pcadach) 2003-12-23 22:02:38.000-0600

Jeremy, could you explain what is wrong with this patch more detailed?

As for me (and for mdu113) it works very stable and passes audio back to caller.
By: jerjer (jerjer) 2003-12-23 22:05:40.000-0600

it segfaults on incoming H.323 calls on

     if(strlen(p->cd.call_redir_e164))
             strncpy(p->rdnis, cd.call_redir_e164, sizeof(p->rdnis)-1);

(search for "exit:")

Then the new debug you added comes out as garbage
By: Paul Cadach (pcadach) 2003-12-23 22:49:07.000-0600

Ok, I'll clean-up some issues.

H-m-m-m, I've never get sigfauls on incoming calls. Check MyH323Connection::OnReceivedSignalSetup() for next lines:
       if(!setupPDU.GetQ931().GetRedirectingNumber(redirE164))
               redirE164 = "";
       cd.call_redir_e164              = (const char *)redirE164;
So, cd.call_redir_e164 will never be NULL or have invalid value.

Looks like patch is applied incorrectly or something goes wrong...

edited on: 12-23-03 22:42
By: jerjer (jerjer) 2003-12-23 22:54:35.000-0600

I did not get any error when applying this diff.  Stop whining and post another diff fixing this problem
By: Paul Cadach (pcadach) 2003-12-23 23:04:05.000-0600

Ok, I'll re-build current CVS tree and check it. Patch will be available approximately at the end of this week.

Have nice holidays!


WBR,
Paul.
By: jerjer (jerjer) 2004-01-10 20:41:58.000-0600

diff is incorrect. Open a new bug with a proper diff after testing.
By: Digium Subversion (svnbot) 2008-01-15 14:39:27.000-0600

Repository: asterisk
Revision: 1877

U   trunk/channels/chan_h323.c
U   trunk/channels/h323/ast_h323.cpp
U   trunk/channels/h323/ast_h323.h
U   trunk/channels/h323/chan_h323.h
U   trunk/channels/h323/h323.conf.sample

------------------------------------------------------------------------
r1877 | jeremy | 2008-01-15 14:39:27 -0600 (Tue, 15 Jan 2008) | 2 lines

Apply massive patch from PCadach. If things are broken blame him. ASTERISK-465

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=1877