Re: [Opalvoip-user] Bug in OpalRFC2833Proto::SendToneAsync Opal 3.12.9

[David J. <dav...@se...>]

Hello Robert,

Sorry but I have to disagree here.

I have attached four files to this mail.
- "send_success.log" contains the log output when the sending of a tone
succeeds
- "send_failed.log" contains the log when it fails
- "debug.patch" contains a patch that adds a little debug output to
better visualise the problem
- "rfc2833.cxx" contains the patched source file (as a reference for the
mentioned line numbers in the following text)


So here is what happens in my opinion:

- OpalRFC2833Proto::SendToneAsync() is called
  - m_transmitState is idle, so the if block on line 277 is entered
    - m_transmitCode gets the new tone to send
    - m_transmitState is set to TransmitActive
  - Because I specifiy a duration when invoking this function, the else
block on line 291 is entered
    - Now the m_asyncDurationTimer is reset (also shown in the log that
was added through the patch)

At the end of SendToneAsync() OpalRFC2833Proto::SendAsyncFrame() is
called (line 299):
- The switch on line 332 checks the m_transmitState
- case TransmitActive is entered (shown in the log)

Now we are at the critical line; The if on line 336 "if
(m_asyncDurationTimer.IsRunning())" checks whether the timer is running.
- If it is running, the transmission of the tone starts correctly (as
seen in "send_success.log")
- If it is not running, the transmissions is not started and the
m_transmitState is set to TransmitEnding1 (as seen in
"send_failed.log"). The transmission ends _before_ it started.

As I described in my previous mail, this is a race condition due to the
multi threaded nature of the housekeeper and the timers.

Is my analysis correct or am I missing something?


Kind regards
David


On 12/07/2014 04:14 AM, Robert Jongbloed wrote:
> I am sorry, but I really do not think that is the problem. Your issue
> may be fixed by a short delay, but there is no logical reason for
> waiting till the m_asyncDurationTimer time has started would have any
> affect on SendAsyncFrame() as it never examines that variable, it only
> stops it under in error cases, so how can it not having been started
> yet affect if the packet is transmitted?
>
> There is something else going on, and I think you might need to dig a
> little deeper to find it. The short delay may be a clue, do you get
> the "No RTP session suitable" or "No payload type to send packet"
> messages? If not you mighty consider added a log to:
>
>    if (m_transmitState == TransmitIdle) {
>
> to see if it going out that path.
>
>
> Robert Jongbloed
> /OPAL/OpenH323/PTLib Architect and Co-founder/.
>
>
> On 28 October 2014 at 00:23, David Jaschuk <dav...@se...
> <mailto:dav...@se...>> wrote:
>
>     Hello,
>
>     I am using Opal 3.12.9.
>
>     There is a bug in the function OpalRFC2833Proto::SendToneAsync in
>     (src/codec/rfc2833.cxx). I am not sure if this is a known bug, so
>     i just
>     wanted to inform you:
>
>     On line 293, the m_asyncDurationTimer is reset with a new value. This
>     sets the timer in one-shot mode and queues it to be started by the
>     housekeeper. Unfortunately, the housekeeper might be not fast
>     enough to
>     start the timer before it is used in OpalRFC2833Proto::SendAsyncFrame
>     (line 334).
>     This results in no sent DTMF packets.
>
>     I did a quickfix with the attached patch, but am aware that there
>     might
>     be better ways of fixing this (e.g. by using some kind of mutex).
>
>
>     Kind regards
>     David
>
>     ------------------------------------------------------------------------------
>
>     _______________________________________________
>     Opalvoip-user mailing list
>     Opa...@li...
>     <mailto:Opa...@li...>
>     https://lists.sourceforge.net/lists/listinfo/opalvoip-user
>
>

-- 
Dipl.-Inf
David Jaschuk
Berater
Entwicklung Client- & Netzwerksicherheit
Public Sector
secunet Security Networks AG
Alt-Moabit 91c
10559 Berlin
dav...@se...
http://www.secunet.com
Public Sector
secunet Security Networks AG 

______________________________________________________________________

Sitz: Kronprinzenstraße 30, 45128 Essen, Deutschland
Amtsgericht Essen HRB 13615
Vorstand: Dr. Rainer Baumgart (Vors.), Thomas Pleines 
Aufsichtsratsvorsitzender: Dr. Peter Zattler
______________________________________________________________________