zaphoyd / websocketpp

C++ websocket client/server library
http://www.zaphoyd.com/websocketpp
Other
6.96k stars 1.97k forks source link

connection_hdl should support less-than operator if built with android ndk #457

Open lotodore opened 9 years ago

lotodore commented 9 years ago

We are using connection_hdl in a map as follows:

typedef std::map<websocketpp::connection_hdl, boost::weak_ptr<SessionData> > SessionMap;

Usually this works fine, but with the (current) android ndk (gcc 4.8) this causes a build error:

--snip--

In file included from ../../../Programme/android-ndk-r10e/sources/cxx-stl/gnu-libstdc++/4.9/include/functional:49:0,
                 from ../../../Programme/android-ndk-r10e/sources/cxx-stl/gnu-libstdc++/4.9/include/thread:39,
                 from ../pokerth/src/third_party/websocketpp/websocketpp/common/thread.hpp:47,
                 from ../pokerth/src/third_party/websocketpp/websocketpp/concurrency/basic.hpp:31,
                 from ../pokerth/src/third_party/websocketpp/websocketpp/config/core.hpp:37,
                 from ../pokerth/src/third_party/websocketpp/websocketpp/config/asio_no_tls.hpp:31,
                 from ../pokerth/src/net/websocket_defs.h:36,
                 from ../pokerth/src/net/serveracceptwebhelper.h:36,
                 from ../pokerth/src/net/common/serveracceptwebhelper.cpp:32:
../../../Programme/android-ndk-r10e/sources/cxx-stl/gnu-libstdc++/4.9/include/bits/stl_function.h: In instantiation of 'bool std::less<_Tp>::operator()(const _Tp&, const _Tp&) const [with _Tp = std::weak_ptr<void>]':
../../../Programme/android-ndk-r10e/sources/cxx-stl/gnu-libstdc++/4.9/include/bits/stl_tree.h:1955:8:   required from 'std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::iterator std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::find(const _Key&) [with _Key = std::weak_ptr<void>; _Val = std::pair<const std::weak_ptr<void>, boost::weak_ptr<SessionData> >; _KeyOfValue = std::_Select1st<std::pair<const std::weak_ptr<void>, boost::weak_ptr<SessionData> > >; _Compare = std::less<std::weak_ptr<void> >; _Alloc = std::allocator<std::pair<const std::weak_ptr<void>, boost::weak_ptr<SessionData> > >; std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::iterator = std::_Rb_tree_iterator<std::pair<const std::weak_ptr<void>, boost::weak_ptr<SessionData> > >]'
../../../Programme/android-ndk-r10e/sources/cxx-stl/gnu-libstdc++/4.9/include/bits/stl_map.h:860:29:   required from 'std::map<_Key, _Tp, _Compare, _Alloc>::iterator std::map<_Key, _Tp, _Compare, _Alloc>::find(const key_type&) [with _Key = std::weak_ptr<void>; _Tp = boost::weak_ptr<SessionData>; _Compare = std::less<std::weak_ptr<void> >; _Alloc = std::allocator<std::pair<const std::weak_ptr<void>, boost::weak_ptr<SessionData> > >; std::map<_Key, _Tp, _Compare, _Alloc>::iterator = std::_Rb_tree_iterator<std::pair<const std::weak_ptr<void>, boost::weak_ptr<SessionData> > >; std::map<_Key, _Tp, _Compare, _Alloc>::key_type = std::weak_ptr<void>]'
../pokerth/src/net/common/serveracceptwebhelper.cpp:103:50:   required from here
../../../Programme/android-ndk-r10e/sources/cxx-stl/gnu-libstdc++/4.9/include/bits/stl_function.h:367:20: error: no match for 'operator<' (operand types are 'const std::weak_ptr<void>' and 'const std::weak_ptr<void>')
       { return __x < __y; }
                    ^
../../../Programme/android-ndk-r10e/sources/cxx-stl/gnu-libstdc++/4.9/include/bits/stl_function.h:367:20: note: candidates are:
In file included from ../../../Programme/android-ndk-r10e/sources/cxx-stl/gnu-libstdc++/4.9/include/set:62:0,
                 from ../../../Programme/cmoss_armv5/bin/droid/include/boost/detail/container_fwd.hpp:95,
                 from ../../../Programme/cmoss_armv5/bin/droid/include/boost/functional/hash/detail/container_fwd_0x.hpp:9,
                 from ../../../Programme/cmoss_armv5/bin/droid/include/boost/functional/hash/extensions.hpp:17,
                 from ../../../Programme/cmoss_armv5/bin/droid/include/boost/functional/hash/hash.hpp:508,
                 from ../../../Programme/cmoss_armv5/bin/droid/include/boost/functional/hash.hpp:6,
                 from ../../../Programme/cmoss_armv5/bin/droid/include/boost/thread/detail/thread.hpp:33,
                 from ../../../Programme/cmoss_armv5/bin/droid/include/boost/thread/thread.hpp:22,
                 from ../../../Programme/cmoss_armv5/bin/droid/include/boost/thread.hpp:13,
                 from ../pokerth/src/net/sessiondata.h:40,
                 from ../pokerth/src/net/sessionmanager.h:39,
                 from ../pokerth/src/net/serverlobbythread.h:41,
                 from ../pokerth/src/net/serveracceptwebhelper.h:38,
                 from ../pokerth/src/net/common/serveracceptwebhelper.cpp:32:
../../../Programme/android-ndk-r10e/sources/cxx-stl/gnu-libstdc++/4.9/include/bits/stl_multiset.h:794:5: note: template<class _Key, class _Compare, class _Alloc> bool std::operator<(const std::multiset<_Key, _Compare, _Alloc>&, const std::multiset<_Key, _Compare, _Alloc>&)
     operator<(const multiset<_Key, _Compare, _Alloc>& __x,

--snip--

It should not be necessary to declare our own comparison operator because one should be able to use a handle as kind of id which implies using it as key in a mapping. With all other environments we have available (even older compilers), this seems to work fine.

lotodore commented 9 years ago

Please note: This works fine with an older release (< 0.5.0) of websocket++ (with the current android ndk).

If the solution is to use std::owner_less, I'd generelly consider this a bad idea. If the handles are weak_ptrs (I hope they are not, but it looks like they are :-) ), then the ordering value will basically change once the pointer is no longer valid. I do not think that this is what anyone wants to happen in a mapping of handles.

zaphoyd commented 9 years ago

connection_hdl is and has always been a weak pointer. std::owner_less orders by the address of the shared pointer control block, not the address pointed to by the shared pointer. The address of the control block is stable and valid as long as at least one weak pointer remains. If you have a connection_hdl then you are guaranteed to have at least one weak pointer. This also ensures that connection_hdl's never overlap, overflow, or recycle until the prior user of that address is done with it.

Prior to 0.5.0 WebSocket++ used boost::weak_pointer by default and std::weak_pointer only if a special C++11 mode flag was defined. Starting in 0.5.0, C++11 library availability is automatically detected on most popular platforms when the -std=c++0x/11 or later language dialect is selected. There are some more exotic compilers and platforms that cannot be autodetected and may fall back to boost unless C++11 mode is explicitly turned on.

I should probably look into seeing if there is a way for the library to explicitly supply comparison operators for cases when std::weak_pointer is used so end users don't have to define this themselves and the transition between boost and std based connection_hdls is more smooth.

lotodore commented 9 years ago

OK sorry, I was obviously wrong about std::owner_less with weak_ptr. I did not know it behaves that way, feels somehow counter-intuitive to its name. We'll try to solve the problem using owner_less, but still, I think this is unecessary complexity. The user of the lib should not need to know that connection_hdl is a weak_ptr and it should work with normal STL containers. Even a native FILE handle does that...