Re: [reSIProcate] WSS with AfterSocketCreationFunction causes core
The reason I tried it this way, I notice that Transport.hxx doesn't use
the virtual keyword for similar things, e.g. it contains:
int port() const { return mTuple.getPort(); }
bool isV4() const { return mTuple.isV4(); } // !dcm! -- deprecate ASAP
IpVersion ipVersion() const { return mTuple.ipVersion(); }
const sockaddr& boundInterface() const { return mTuple.getSockaddr(); }
but as mentioned, I'm happy to revise this further if it is troublesome
On 18/06/13 19:46, Daniel Pocock 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@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
>>>>
>>>