[reSIProcate] WSS with AfterSocketCreationFunction causes core
Scott Godin
sgodin at sipspectrum.com
Tue Jun 18 13:32:32 CDT 2013
Fix seems fine to me. You've retained the transport() method in
Transport.hxx so API users should not notice anything missing. I would
remove the signatures from the other header files though to cleanup the
code, instead of leaving them commented out.
I like adding dummy certs.
Scott
On Tue, Jun 18, 2013 at 1:46 PM, Daniel Pocock <daniel at pocock.com.au> wrote:
>
>
> I've just committed the variation of the fix removing virtual, but I'm
> happy to revise this later if there are problems with it.
>
> I've also commented out the TLS and WSS parts of the test case because
> they require certs - do you think we should include dummy certs in the
> source tree or just script the openssl command to make them on the fly?
> I've put the openssl command in a comment in the test case:
>
>
> https://svn.resiprocate.org/viewsvn/resiprocate/main/resip/stack/test/testSocketFunc.cxx?view=markup#l37
>
>
> On 18/06/13 19:14, Daniel Pocock wrote:
> >
> >
> > On 18/06/13 15:13, Scott Godin wrote:
> >> In InternalTransport::bind - we could access the transport type from
> mTuple
> >> instead - it is set in the constructor of each derived class before
> bind is
> >> called (from init()).
> >>
> >> if (mSocketFunc)
> >> {
> >> mSocketFunc(mFd, mTuple.getType(), __FILE__, __LINE__);
> >> }
> >
> >
> > Oddly enough, TlsTransport sets mTuple with this superclass constructor
> > invocation (from TlsTransport.cxx):
> >
> > TlsBaseTransport(fifo, portNum, version, interfaceObj, security,
> > sipDomain, sslType, transport(), socketFunc, compression,
> > transportFlags, cvm, useEmailAsSIP)
> >
> >
> > Notice it is calling the virtual function transport() to get the value?
> >
> > and in TcpTransport.cxx, it is slightly different, the constructor
> > contains this call:
> >
> > mTuple.setType(transport());
> >
> > but that is still naughty too.
> >
> > What I'm now tempted to do is
> >
> > a) remove the virtual keyword from transport()'s prototype, and replace
> > it with { return mTuple.getType(); }
> >
> > b) change all other superclass constructor invocations to have hardcoded
> > transport IDs to that (a) doesn't lead to unpleasant recursion
> >
> >
> >>
> >> Scott
> >>
> >> On Mon, Jun 17, 2013 at 4:48 PM, Daniel Pocock <daniel at pocock.com.au>
> wrote:
> >>
> >>>
> >>>
> >>> On 15/06/13 22:36, Vladimir Pavluk wrote:
> >>>> Hi Daniel,
> >>>>
> >>>>> I've had a quick look at this
> >>>>>
> >>>>> I notice that in repro we call SipStack::addTransport( .. ) and we
> don't
> >>>>> set mSocketFunc
> >>>>>
> >>>>> In your test case, you use DialogUsageManager::addTransport( .. )
> which
> >>>>> just proxies the call to mStack.addTransport
> >>>>>
> >>>>> Do you definitely need to use the dum variation? I think it may be
> >>>>> useful to have this test case under resip/stack and avoid the dum
> >>>>> dependency if possible.
> >>>> No matter whichever we use (and I think we actually use
> >>>> SipStack::addTransport( .. ) in our app), it always cores.
> >>>>
> >>>>> Would you know if it cores for TLS, or only for WSS?
> >>>> It does core for both (you can build the test program and make sure).
> >>>>
> >>>>> Could you possibly share a stack trace? You can exclude those parts
> of
> >>>>> the stack that are in your own code.
> >>>> It aborts with "pure virtual function call" right on the line where it
> >>>> calls AfterSocketCreationFunction (we'll get back to you with more
> >>>> precise stack trace).
> >>>>
> >>>
> >>> I've adapted the test case and confirmed there is an issue (see below)
> >>>
> >>> The root cause is that it calls a pure virtual function,
> >>> Transport::transport() during construction of an object of class
> >>> TlsTransport
> >>>
> >>> I can confirm that this is definitely bad behavior. I hacked around in
> >>> this code a little bit to enable support for WSS, but not at this low
> >>> level where the virtual function exists, so I don't believe it is a
> >>> regression.
> >>>
> >>> Oddly, creating a TcpTransport invokes the same code path, it doesn't
> >>> trigger this problem, but it is not safe for it to rely on the virtual
> >>> function call during construction either.
> >>>
> >>> One option that comes to mind is that we can pass the transport type
> >>> down from the constructor to make sure it is initialized. The other
> >>> option is to defer initialisation until after construction.
> >>>
> >>>
> >>>
> >>> $ libtool --mode=execute gdb resip/stack/test/testSocketFunc
> >>> GNU gdb (GDB) 7.4.1-debian
> >>> Copyright (C) 2012 Free Software Foundation, Inc.
> >>> License GPLv3+: GNU GPL version 3 or later
> >>> <http://gnu.org/licenses/gpl.html>
> >>> This is free software: you are free to change and redistribute it.
> >>> There is NO WARRANTY, to the extent permitted by law. Type "show
> copying"
> >>> and "show warranty" for details.
> >>> This GDB was configured as "x86_64-linux-gnu".
> >>> For bug reporting instructions, please see:
> >>> <http://www.gnu.org/software/gdb/bugs/>...
> >>> Reading symbols from
> >>>
> >>>
> /home/daniel/ws/resiprocate/resiprocate-trunk.git/resip/stack/test/.libs/lt-testSocketFunc...done.
> >>> (gdb) run
> >>> Starting program:
> >>>
> >>>
> /home/daniel/ws/resiprocate/resiprocate-trunk.git/resip/stack/test/.libs/lt-testSocketFunc
> >>>
> >>> [Thread debugging using libthread_db enabled]
> >>> Using host libthread_db library
> "/lib/x86_64-linux-gnu/libthread_db.so.1".
> >>> INFO | 20130617-221954.755 | | RESIP:DNS | 140737353914144 |
> >>> dns/AresDns.cxx:394 | DNS initialization: using c-ares v1.9.1
> >>> INFO | 20130617-221954.755 | | RESIP:DNS | 140737353914144 |
> >>> dns/AresDns.cxx:403 | DNS initialization: found 1 name servers
> >>> INFO | 20130617-221954.755 | | RESIP:DNS | 140737353914144 |
> >>> dns/AresDns.cxx:409 | name server: 192.168.1.2
> >>> INFO | 20130617-221954.760 | | RESIP:TRANSPORT | 140737353914144 |
> >>> Connection.cxx:38 | Connection::Connection: new connection created to
> >>> who: [ V4 0.0.0.0:0 UNKNOWN_TRANSPORT target domain=unspecified
> >>> mFlowKey=0 ]
> >>> pure virtual method called
> >>> terminate called without an active exception
> >>>
> >>> Program received signal SIGABRT, Aborted.
> >>> 0x00007ffff6185475 in raise () from /lib/x86_64-linux-gnu/libc.so.6
> >>> (gdb) bt
> >>> #0 0x00007ffff6185475 in raise () from /lib/x86_64-linux-gnu/libc.so.6
> >>> #1 0x00007ffff61886f0 in abort () from /lib/x86_64-linux-gnu/libc.so.6
> >>> #2 0x00007ffff69da89d in __gnu_cxx::__verbose_terminate_handler() ()
> >>> from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
> >>> #3 0x00007ffff69d8996 in ?? () from
> >>> /usr/lib/x86_64-linux-gnu/libstdc++.so.6
> >>> #4 0x00007ffff69d89c3 in std::terminate() () from
> >>> /usr/lib/x86_64-linux-gnu/libstdc++.so.6
> >>> #5 0x00007ffff69d94df in __cxa_pure_virtual () from
> >>> /usr/lib/x86_64-linux-gnu/libstdc++.so.6
> >>> #6 0x00007ffff7ab7d3e in resip::InternalTransport::bind
> (this=0x696e80)
> >>> at InternalTransport.cxx:147
> >>> #7 0x00007ffff7afde98 in resip::TcpBaseTransport::init (this=0x696e80)
> >>> at TcpBaseTransport.cxx:82
> >>> #8 0x00007ffff7b53559 in resip::TlsBaseTransport::TlsBaseTransport
> >>> (this=0x696e80, fifo=..., portNum=<optimized out>, version=<optimized
> >>> out>, interfaceObj=..., security=...,
> >>> sipDomain=..., sslType=resip::SecurityTypes::TLSv1,
> >>> transportType=resip::TLS, socketFunc=0x401230
> >>> <afterSocketCreationFunction(int, int, char const*, int)>,
> compression=...,
> >>> transportFlags=0, cvm=resip::SecurityTypes::None,
> >>> useEmailAsSIP=false) at ssl/TlsBaseTransport.cxx:46
> >>> #9 0x00007ffff7b564a3 in resip::TlsTransport::TlsTransport
> >>> (this=0x696e80, fifo=..., portNum=<optimized out>, version=<optimized
> >>> out>, interfaceObj=..., security=..., sipDomain=...,
> >>> sslType=resip::SecurityTypes::TLSv1, socketFunc=0x401230
> >>> <afterSocketCreationFunction(int, int, char const*, int)>,
> >>> compression=..., transportFlags=0, cvm=resip::SecurityTypes::None,
> >>> useEmailAsSIP=false) at ssl/TlsTransport.cxx:35
> >>> #10 0x00007ffff7af75f2 in resip::SipStack::addTransport
> >>> (this=0x7ffffffb6660, protocol=resip::TLS, port=5061,
> version=resip::V4,
> >>> stun=resip::StunDisabled, ipInterface=...,
> >>> sipDomainname=..., privateKeyPassPhrase=...,
> >>> sslType=resip::SecurityTypes::TLSv1, transportFlags=0,
> >>> cvm=resip::SecurityTypes::None, useEmailAsSIP=false) at
> SipStack.cxx:359
> >>> #11 0x000000000040105f in main () at testSocketFunc.cxx:41
> >>> _______________________________________________
> >>> resiprocate-devel mailing list
> >>> resiprocate-devel at resiprocate.org
> >>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
> >>>
> >>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20130618/fd00ba75/attachment.htm>
More information about the resiprocate-devel
mailing list