[h323plus] Twice H323Connection::Unlock() call issue
Jan Willamowius
jan at willamowius.de
Fri Jan 22 13:38:46 EST 2016
Hi Iurii,
I have put a fix similar to what you suggested into the CVS.
Thanks for posting all these bug reports! I'll try to slowly work my
way through them; Simon seems to be busy.
It would help me if you could post your suggested changes as "diff -u"
against the CVS.
Regards,
Jan
--
Jan Willamowius, Founder of the GNU Gatekeeper Project
EMail : jan at willamowius.de
Website: http://www.gnugk.org
Support: http://www.willamowius.com/gnugk-support.html
Relaxed Communications GmbH
Frahmredder 91
22393 Hamburg
Geschäftsführer: Jan Willamowius
HRB 125261 (Amtsgericht Hamburg)
USt-IdNr: DE286003584
Iurii Gordiienko wrote:
> Hi Jan,
> I think, your solution will work, but in my opinion we have to fix H323Connection::SendSignalSetup function, because it (only) has strange behavior. It has different internal state (locked and unlocked) for the same return reason (Not H323Connection::EndedByCallerAbort).
> Take a look to my proposition (a bit draft variant but in anyway) - in worst case (if not able to Lock the mutexes) we only lost the real reason (why does it fails) and return EndedByCallerAbort.
> H323Connection::SendSignalSetup
> { CallEndReason reason = NumCallEndReasons;
>
>
> //...
> // Do the transport connect connectionState = AwaitingTransportConnect;
> // Release the mutex as can deadlock trying to clear call during connect. Unlock();
> PBoolean connectFailed = false; if (!signallingChannel->IsOpen()) { signallingChannel->SetWriteTimeout(100); connectFailed = !signallingChannel->Connect(); }
> // See if transport connect failed, abort if so. if (connectFailed) { connectionState = NoConnectionActive; switch (signallingChannel->GetErrorNumber()) { case ENETUNREACH : reason = EndedByUnreachable; break; case ECONNREFUSED : reason = EndedByNoEndPoint; break; case ETIMEDOUT : reason = EndedByHostOffline; break; default: reason = EndedByConnectFail; } }
> }
> // Lock while checking for shutting down. if (!Lock()) return EndedByCallerAbort; else if(NumCallEndReasons != reason) return reason;
> //...
> return reason; }
> Thanks -- Iurii Gordiienko
>
> --- Original message ---
> From: "Jan Willamowius" <jan at willamowius.de>
> Date: 27 December 2015, 19:47:21
>
> Hi Iurii,
>
> PTLib 2.10.x and 2.11.x have a method to check, if a mutex is already
> locked, but they changed the API and from 2.12.x on there is no way to
> check except to try to lock it.
>
> Replace H323Connection::Unlock() in src/h323.cxx with this version:
>
> void H323Connection::Unlock()
> {
> // calling Signal() on a mutex that isn't locked is causing an assert,
> // so we make sure it is locked before the Signal()
> innerMutex.Wait(0);
> innerMutex.Signal();
> outerMutex.Wait(0);
> outerMutex.Signal();
> }
>
> Please try the change and report back how it works.
> Then I'll commit the code to the CVS.
>
> Regards,
> Jan
>
> --
> Jan Willamowius, Founder of the GNU Gatekeeper Project
> EMail : jan at willamowius.de
> Website: http://www.gnugk.org
> Support: http://www.willamowius.com/gnugk-support.html
>
> Relaxed Communications GmbH
> Frahmredder 91
> 22393 Hamburg
> Geschäftsführer: Jan Willamowius
> HRB 125261 (Amtsgericht Hamburg)
> USt-IdNr: DE286003584
>
>
> Iurii Gordiienko wrote:
> > Hi,
> > I have found a bug with twice H323Connection::Unlock() call if we have an connection issue.
> > Take a look:
> > 1.We are running connection.SendSignalSetup function from H225CallThread::Main() and if it returns any reason!=H323Connection::EndedByCallerAbort we will Unlock the connection.
> >
> > void H225CallThread::Main() { PTRACE(3, "H225\tStarted call thread");
> > if (connection.Lock()) { H323Connection::CallEndReason reason = connection.SendSignalSetup(alias, address);
> > // Special case, if we aborted the call then already will be unlocked if (reason != H323Connection::EndedByCallerAbort) connection.Unlock();
> > // bla-bla-bla }
> >
> > 2.But we have case when connection.SendSignalSetup returns non H323Connection::EndedByCallerAbort reason but has own unlocked state - for this case PTLIB will unlock the mutex twice with assert (as minimum) message.
> > H323Connection::SendSignalSetup(const PString & alias,
> > const H323TransportAddress & address) { //bla-bla-bla
> > // Release the mutex as can deadlock trying to clear call during connect. Unlock();
> > PBoolean connectFailed = false; if (!signallingChannel->IsOpen()) { signallingChannel->SetWriteTimeout(100); connectFailed = !signallingChannel->Connect(); }
> > // See if transport connect failed, abort if so. if (connectFailed) { connectionState = NoConnectionActive; switch (signallingChannel->GetErrorNumber()) { case ENETUNREACH : return EndedByUnreachable; case ECONNREFUSED : return EndedByNoEndPoint; case ETIMEDOUT : return EndedByHostOffline; } return EndedByConnectFail; }
> > }
> > Thanks -- Iurii Gordiienko
>
More information about the h323plus
mailing list