Re: [reSIProcate] WSS with AfterSocketCreationFunction causes core
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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>
>