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@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
>>>
>>