Initialization order fiasco in some data #71

oxidase · 2018-06-13T00:17:20Z

Running valhalla node bindings with enabled address sanitizer and linked with prime_server crashes due to incorrect initialization order of static structures:

Thread 1 "node" received signal SIGSEGV, Segmentation fault.
0x00007fffe6cdde95 in std::_Hashtable<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, prime_server::method_t>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, prime_server::method_t> >, std::__detail::_Select1st, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::_Hashtable<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, prime_server::method_t> const*> (this=0x7fffe4c2fb60 <prime_server::STRING_TO_METHOD>, __f=0x7fffffffa6f0, __l=0x7fffffffa830, __bucket_hint=0, __h1=..., __h2=..., __h=..., __eq=..., __exk=..., __a=...) at /usr/include/c++/7/bits/hashtable.h:955
955	      _Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal,
(gdb) bt
#0  0x00007fffe6cdde95 in std::_Hashtable<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, prime_server::method_t>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, prime_server::method_t> >, std::__detail::_Select1st, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::_Hashtable<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, prime_server::method_t> const*> (this=0x7fffe4c2fb60 <prime_server::STRING_TO_METHOD>, __f=0x7fffffffa6f0, __l=0x7fffffffa830, __bucket_hint=0, __h1=..., __h2=..., __h=..., __eq=..., __exk=..., __a=...) at /usr/include/c++/7/bits/hashtable.h:955
#1  0x00007fffe4a0460f in std::_Hashtable<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, prime_server::method_t>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, prime_server::method_t> >, std::__detail::_Select1st, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::_Hashtable (__a=..., __eql=..., __hf=..., __n=0, __l=..., this=0x7fffe4c2fb60 <prime_server::STRING_TO_METHOD>) at /usr/include/c++/7/bits/hashtable.h:453
#2  std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, prime_server::method_t, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, prime_server::method_t> > >::unordered_map (__a=..., __eql=..., __hf=..., __n=0, __l=..., this=0x7fffe4c2fb60 <prime_server::STRING_TO_METHOD>) at /usr/include/c++/7/bits/unordered_map.h:230
#3  __static_initialization_and_destruction_0 (__priority=65535, __initialize_p=1) at ./prime_server/http_protocol.hpp:57
#4  0x00007ffff7de5733 in call_init (env=0x7fffffffdef0, argv=0x7fffffffded8, argc=2, l=<optimized out>) at dl-init.c:72
#5  _dl_init (main_map=main_map@entry=0x226e090, argc=2, argv=0x7fffffffded8, env=0x7fffffffdef0) at dl-init.c:119
#6  0x00007ffff7dea1ff in dl_open_worker (a=a@entry=0x7fffffffacb0) at dl-open.c:522
#7  0x00007ffff6bdc2df in __GI__dl_catch_exception (exception=0x7fffffffac90, operate=0x7ffff7de9dc0 <dl_open_worker>, args=0x7fffffffacb0) at dl-error-skeleton.c:196
#8  0x00007ffff7de97ca in _dl_open (file=0x7fffffffb398 "/home/miha/valhalla/lib/binding/node_valhalla.node", mode=-2147483647, caller_dlopen=0x1440a27 <uv_dlopen+39>, nsid=<optimized out>, argc=2, argv=<optimized out>, env=0x7fffffffdef0) at dl-open.c:605
#9  0x00007ffff7bd1f96 in dlopen_doit (a=a@entry=0x7fffffffaee0) at dlopen.c:66
#10 0x00007ffff6bdc2df in __GI__dl_catch_exception (exception=exception@entry=0x7fffffffae80, operate=0x7ffff7bd1f40 <dlopen_doit>, args=0x7fffffffaee0) at dl-error-skeleton.c:196
#11 0x00007ffff6bdc36f in __GI__dl_catch_error (objname=0x22658d0, errstring=0x22658d8, mallocedp=0x22658c8, operate=<optimized out>, args=<optimized out>) at dl-error-skeleton.c:215
#12 0x00007ffff7bd2735 in _dlerror_run (operate=operate@entry=0x7ffff7bd1f40 <dlopen_doit>, args=args@entry=0x7fffffffaee0) at dlerror.c:162
#13 0x00007ffff7bd2051 in __dlopen (file=file@entry=0x7fffffffb398 "/home/miha/valhalla/lib/binding/node_valhalla.node", mode=mode@entry=1) at dlopen.c:87
#14 0x0000000001440a27 in uv_dlopen (filename=0x7fffffffb398 "/home/miha/valhalla/lib/binding/node_valhalla.node", lib=0x7fffffffaf50) at ../deps/uv/src/unix/dl.c:36
#15 0x000000000121f245 in node::DLOpen(v8::FunctionCallbackInfo<v8::Value> const&) ()
#16 0x0000000000b1a673 in v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) ()
#17 0x0000000000b9043c in v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) ()
#18 0x0000000000b9108f in v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) ()

and

#0  0x00007fffe6cf96a9 in __gnu_cxx::__to_xstring<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, char> (__convf=0x83bbd0 <vsnprintf@plt>, __n=32, __fmt=0x7fffe4a6ca44 "%lu") at /usr/include/c++/7/ext/string_conversions.h:99
#1  0x00007fffe4a5d582 in std:: (__val=<optimized out>) at /usr/include/c++/7/bits/basic_string.h:6421
#2  prime_server::netstring_entity_t::to_string (body="BAD_REQUEST: Non-numeric length") at src/netstring_protocol.cpp:36
#3  0x00007fffe4a5d789 in prime_server::netstring_entity_t::request_exception_t::request_exception_t (this=<optimized out>, response=...) at src/netstring_protocol.cpp:135
#4  0x00007fffe4a4aa59 in __static_initialization_and_destruction_0 (__priority=65535, __initialize_p=1) at src/netstring_protocol.cpp:12
#5  0x00007ffff7de5733 in call_init (env=0x7fffffffdef0, argv=0x7fffffffded8, argc=2, l=<optimized out>) at dl-init.c:72
#6  _dl_init (main_map=main_map@entry=0x226e090, argc=2, argv=0x7fffffffded8, env=0x7fffffffdef0) at dl-init.c:119
#7  0x00007ffff7dea1ff in dl_open_worker (a=a@entry=0x7fffffffacb0) at dl-open.c:522
#8  0x00007ffff6bdc2df in __GI__dl_catch_exception (exception=0x7fffffffac90, operate=0x7ffff7de9dc0 <dl_open_worker>, args=0x7fffffffacb0) at dl-error-skeleton.c:196
#9  0x00007ffff7de97ca in _dl_open (file=0x7fffffffb398 "/home/miha/valhalla/lib/binding/node_valhalla.node", mode=-2147483647, caller_dlopen=0x1440a27 <uv_dlopen+39>, nsid=<optimized out>, argc=2, argv=<optimized out>, env=0x7fffffffdef0) at dl-open.c:605
#10 0x00007ffff7bd1f96 in dlopen_doit (a=a@entry=0x7fffffffaee0) at dlopen.c:66
#11 0x00007ffff6bdc2df in __GI__dl_catch_exception (exception=exception@entry=0x7fffffffae80, operate=0x7ffff7bd1f40 <dlopen_doit>, args=0x7fffffffaee0) at dl-error-skeleton.c:196
#12 0x00007ffff6bdc36f in __GI__dl_catch_error (objname=0x226fd90, errstring=0x226fd98, mallocedp=0x226fd88, operate=<optimized out>, args=<optimized out>) at dl-error-skeleton.c:215
#13 0x00007ffff7bd2735 in _dlerror_run (operate=operate@entry=0x7ffff7bd1f40 <dlopen_doit>, args=args@entry=0x7fffffffaee0) at dlerror.c:162
#14 0x00007ffff7bd2051 in __dlopen (file=file@entry=0x7fffffffb398 "/home/miha/valhalla/lib/binding/node_valhalla.node", mode=mode@entry=1) at dlopen.c:87
#15 0x0000000001440a27 in uv_dlopen (filename=0x7fffffffb398 "/home/miha/valhalla/lib/binding/node_valhalla.node", lib=0x7fffffffaf50) at ../deps/uv/src/unix/dl.c:36
#16 0x000000000121f245 in node::DLOpen(v8::FunctionCallbackInfo<v8::Value> const&) ()
#17 0x0000000000b1a673 in v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) ()
#18 0x0000000000b9043c in v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) ()
#19 0x0000000000b9108f in v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) ()

The first crash is fixed by the patch

fix-init-order master 952adbe4201f9b166f812d8c2d1a13ff3f4137b7
Author:     Michael Krasnyk <[email protected]>
AuthorDate: Wed Jun 13 01:11:13 2018 +0200
Commit:     Michael Krasnyk <[email protected]>
CommitDate: Wed Jun 13 01:11:13 2018 +0200

Parent:     9324d4f cat the right things on failure, include the right headers for installation
Merged:     fix-init-order master
Containing: fix-init-order master
Follows:    0.6.3 (44)

Fix initialization order fiasco of unordered maps

2 files changed, 31 insertions(+), 20 deletions(-)
prime_server/http_protocol.hpp | 11 -----------
src/http_protocol.cpp          | 40 +++++++++++++++++++++++++++++++---------

modified   prime_server/http_protocol.hpp
@@ -51,17 +51,6 @@ namespace prime_server {
   using headers_t = std::unordered_map<std::string, std::string, caseless_predicates_t, caseless_predicates_t>;
   using query_t = std::unordered_map<std::string, std::list<std::string> >;
   enum method_t { OPTIONS, GET, HEAD, POST, PUT, DELETE, TRACE, CONNECT };
-  const std::unordered_map<std::string, method_t> STRING_TO_METHOD {
-    {"OPTIONS", method_t::OPTIONS}, {"GET", method_t::GET}, {"HEAD", method_t::HEAD}, {"POST", method_t::POST},
-    {"PUT", method_t::PUT}, {"DELETE", method_t::DELETE}, {"TRACE", method_t::TRACE}, {"CONNECT", method_t::CONNECT}
-  };
-  const std::unordered_map<method_t, std::string, std::hash<int> > METHOD_TO_STRING{
-    {method_t::OPTIONS, "OPTIONS"}, {method_t::GET, "GET"}, {method_t::HEAD, "HEAD"}, {method_t::POST, "POST"},
-    {method_t::PUT, "PUT"}, {method_t::DELETE, "DELETE"}, {method_t::TRACE, "TRACE"}, {method_t::CONNECT, "CONNECT"}
-  };
-  const std::unordered_map<std::string, bool> SUPPORTED_VERSIONS {
-    {"HTTP/1.0", true}, {"HTTP/1.1", true}
-  };
 
   struct http_entity_t {
     std::string version;
modified   src/http_protocol.cpp
@@ -39,6 +39,28 @@ namespace {
   const http_request_t::request_exception_t RESPONSE_504(http_response_t(504, "Gateway Time-out", "The server didn't respond in time", {CORS}));
   const http_request_t::request_exception_t RESPONSE_505(http_response_t(505, "HTTP Version Not Supported", "The HTTP request version is not supported", {CORS}));
 
+
+  const std::unordered_map<std::string, method_t>& string_to_method() {
+    static const std::unordered_map<std::string, method_t> data {
+       {"OPTIONS", method_t::OPTIONS}, {"GET", method_t::GET}, {"HEAD", method_t::HEAD}, {"POST", method_t::POST},
+       {"PUT", method_t::PUT}, {"DELETE", method_t::DELETE}, {"TRACE", method_t::TRACE}, {"CONNECT", method_t::CONNECT}
+    };
+    return data;
+  }
+  const std::unordered_map<method_t, std::string, std::hash<int> >& method_to_string() {
+    static const std::unordered_map<method_t, std::string, std::hash<int> > data {
+      {method_t::OPTIONS, "OPTIONS"}, {method_t::GET, "GET"}, {method_t::HEAD, "HEAD"}, {method_t::POST, "POST"},
+      {method_t::PUT, "PUT"}, {method_t::DELETE, "DELETE"}, {method_t::TRACE, "TRACE"}, {method_t::CONNECT, "CONNECT"}
+    };
+    return data;
+  }
+  const std::unordered_map<std::string, bool>& supported_versions() {
+    static const std::unordered_map<std::string, bool> data {
+      {"HTTP/1.0", true}, {"HTTP/1.1", true}
+    };
+    return data;
+  }
+
   template <class T>
   size_t name_max(const std::unordered_map<std::string, T>& methods) {
     size_t i = 0;
@@ -46,8 +68,8 @@ namespace {
       i = std::max(i, kv.first.size());
     return i;
   };
-  const size_t METHOD_MAX_SIZE = name_max(prime_server::STRING_TO_METHOD) + 1;
-  const size_t VERSION_MAX_SIZE = name_max(prime_server::SUPPORTED_VERSIONS) + 2;
+  const size_t METHOD_MAX_SIZE = name_max(string_to_method()) + 1;
+  const size_t VERSION_MAX_SIZE = name_max(supported_versions()) + 2;
 }
 
 namespace prime_server {
@@ -258,8 +280,8 @@ namespace prime_server {
   std::string http_request_t::to_string(const method_t& method, const std::string& path, const std::string& body, const query_t& query,
                                const headers_t& headers, const std::string& version) {
     //get the method on there
-    auto itr = METHOD_TO_STRING.find(method);
-    if(itr == METHOD_TO_STRING.end())
+    auto itr = method_to_string().find(method);
+    if(itr == method_to_string().end())
       throw std::runtime_error("Unsupported http request method");
     std::string request;
     request.reserve(16 + path.size() + headers.size() * 32 + body.size());
@@ -399,8 +421,8 @@ namespace prime_server {
         case CODE:
           throw RESPONSE_500;
         case METHOD: {
-          auto itr = STRING_TO_METHOD.find(partial_buffer);
-          if(itr == STRING_TO_METHOD.end())
+          auto itr = string_to_method().find(partial_buffer);
+          if(itr == string_to_method().end())
             throw RESPONSE_501;
           log_line = partial_buffer + delimiter;
           method = itr->second;
@@ -415,8 +437,8 @@ namespace prime_server {
           break;
         }
         case VERSION: {
-          auto itr = SUPPORTED_VERSIONS.find(partial_buffer);
-          if(itr == SUPPORTED_VERSIONS.end())
+          auto itr = supported_versions().find(partial_buffer);
+          if(itr == supported_versions().end())
             throw RESPONSE_505;
           log_line += partial_buffer;
           version.swap(partial_buffer);
@@ -599,7 +621,7 @@ namespace prime_server {
           break;
         }
         case VERSION: {
-          if(SUPPORTED_VERSIONS.find(partial_buffer) == SUPPORTED_VERSIONS.end())
+          if(supported_versions().find(partial_buffer) == supported_versions().end())
             throw std::runtime_error("Unknown http version");
           //log_line += partial_buffer;
           version.swap(partial_buffer);

The second one is tricky, because exceptions can be initialized before initialization of vsprintf internal data.

The text was updated successfully, but these errors were encountered:

kevinkreiser · 2018-06-13T13:24:17Z

@oxidase this doesnt look too bad. ill clean it up. maybe its a good time to switch prime_server over to a third party dependency in valhalla. i presume it will eventually be removed in favor of just running valhalla under a node express app?

oxidase · 2018-06-13T13:57:24Z

@kevinkreiser it is a minor issue and occurs for very specific conditions. Just want to make it visible if someone hits the same problem.

rinigus · 2018-07-07T07:28:45Z

@oxidase this doesnt look too bad. ill clean it up. maybe its a good time to switch prime_server over to a third party dependency in valhalla. i presume it will eventually be removed in favor of just running valhalla under a node express app?

I hope that you will keep ability to run valhalla without node. I am using it as a server on mobile platform that doesn't have node support and we'll be hit hard if valhalla will switch off this ability. So, please keep prime_server either separate or third-party, but as available interface for valhalla

kevinkreiser · 2018-07-07T14:22:55Z

@rinigus i think we can find something that works here. the valhalla::tyr::actor class has all of the functionality for each API wrapped into a single method. prime_server itself is just an api for making web (and other) servers. So one easy thing we could do is just keep valhalla_service that just wraps actor instead of having loki_worker, thor_worker, odin_worker etc. its a less scalable/distributed/fault tolerant model but i guess nodejs somehow provides this? anyway this would still allow those who dont want node to use valhalla in pure c++ mode. so basically we can still have a valhalla_service binary that we can run without any node bindings non-sense

rinigus · 2018-07-07T17:15:52Z

We are bit branching from the original issue, but let me ask a question regarding actor_t. I maybe (or to put it more correctly, am) interested in dropping full-blown HTTP service of Valhalla and implement it via my own server (libmicrohttpd) that I am currently running as a proxy for Valhalla service (on the top of tiles and geocoding).

From the brief look on the methods of actor_t, it seem to cover all nicely and should allow me to feed it with JSON data and return Valhalla's reply. That would also simplify my dependencies, reduce number of connections on mobile, and probably response time. But, devil is usually in the details. So, would you mind to tell whether actor_t API is documented anywhere? Couldn't find it, though. If it's not, I'll open an issue regarding it on valhalla repo, so others could easily find it when they need to learn about this API.

kevinkreiser · 2018-07-09T13:40:05Z

@rinigus please open an issue in the valhalla repo and I'd be glad to document it further. its basically just if you have a request like:

localhost:8002/route?json={"locations":[{"lat":40.5,"lon":-76.5},{"lat":40.0,"lon":-76.0}],"costing":"auto}

then you call the actor like:

std::string json_response = actor.route(R"({"locations":[{"lat":40.5,"lon":-76.5},{"lat":40.0,"lon":-76.0}],"costing":"auto})");

And that works the same for all of the other apis like matrix and isochrone etc.

oxidase mentioned this issue Jun 13, 2018

[WIP] node bindings using n-api valhalla/valhalla#1341

Merged

4 tasks

kevinkreiser linked a pull request Jan 12, 2020 that will close this issue

fix some of the initialization fiascos #74

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initialization order fiasco in some data #71

Initialization order fiasco in some data #71

oxidase commented Jun 13, 2018

kevinkreiser commented Jun 13, 2018

oxidase commented Jun 13, 2018

rinigus commented Jul 7, 2018

kevinkreiser commented Jul 7, 2018

rinigus commented Jul 7, 2018

kevinkreiser commented Jul 9, 2018

Initialization order fiasco in some data #71

Initialization order fiasco in some data #71

Comments

oxidase commented Jun 13, 2018

kevinkreiser commented Jun 13, 2018

oxidase commented Jun 13, 2018

rinigus commented Jul 7, 2018

kevinkreiser commented Jul 7, 2018

rinigus commented Jul 7, 2018

kevinkreiser commented Jul 9, 2018