fix: configure jsonconst double parser to use std::from_chars (#4360)

The problem: apparently, jsoncons uses strtod by default when parsing doubles. On some platforms (alpine/musl) this function uses lots of stack, which potentially can lead to stack corruption. This PR configures jsoncons to use std::from_chars that is more efficient and less stack hungry. The single include point to consume jsoncons/json.hpp should be "core/json_object.h" Signed-off-by: Roman Gershman <[email protected]>
dragonflydb · Dec 24, 2024 · 01f24da · 01f24da
1 parent 95cd9df
commit 01f24da
Show file tree

Hide file tree

Showing 8 changed files with 19 additions and 17 deletions.
diff --git a/src/core/compact_object.cc b/src/core/compact_object.cc
@@ -20,8 +20,6 @@ extern "C" {
 #include <absl/strings/str_cat.h>
 #include <absl/strings/strip.h>
 
-#include <jsoncons/json.hpp>
-
 #include "base/flags.h"
 #include "base/logging.h"
 #include "base/pod_array.h"

diff --git a/src/core/compact_object_test.cc b/src/core/compact_object_test.cc
@@ -7,8 +7,6 @@
 #include <mimalloc.h>
 #include <xxhash.h>
 
-#include <jsoncons/json.hpp>
-#include <jsoncons_ext/jsonpath/jsonpath.hpp>
 #include <random>
 
 #include "base/gtest.h"

diff --git a/src/core/json/json_object.h b/src/core/json/json_object.h
@@ -4,6 +4,13 @@
 
 #pragma once
 
+#include <version>  // for __cpp_lib_to_chars macro.
+
+// std::from_chars is available in C++17 if __cpp_lib_to_chars is defined.
+#if __cpp_lib_to_chars >= 201611L
+#define JSONCONS_HAS_STD_FROM_CHARS 1
+#endif
+
 #include <jsoncons/json.hpp>
 #include <jsoncons_ext/jsonpath/jsonpath.hpp>
 #include <memory>

diff --git a/src/server/cluster/cluster_config.cc b/src/server/cluster/cluster_config.cc
@@ -2,7 +2,6 @@
 
 #include <absl/container/flat_hash_set.h>
 
-#include <jsoncons/json.hpp>
 #include <optional>
 #include <string_view>
 

diff --git a/src/server/json_family.cc b/src/server/json_family.cc
@@ -9,12 +9,6 @@
 #include <absl/strings/str_join.h>
 #include <absl/strings/str_split.h>
 
-#include <jsoncons/json.hpp>
-#include <jsoncons_ext/jsonpatch/jsonpatch.hpp>
-#include <jsoncons_ext/jsonpath/jsonpath.hpp>
-#include <jsoncons_ext/jsonpointer/jsonpointer.hpp>
-#include <jsoncons_ext/mergepatch/mergepatch.hpp>
-
 #include "absl/cleanup/cleanup.h"
 #include "base/flags.h"
 #include "base/logging.h"
@@ -36,6 +30,12 @@
 #include "server/tiered_storage.h"
 #include "server/transaction.h"
 
+// clang-format off
+#include <jsoncons_ext/jsonpatch/jsonpatch.hpp>
+#include <jsoncons_ext/jsonpointer/jsonpointer.hpp>
+#include <jsoncons_ext/mergepatch/mergepatch.hpp>
+// clang-format on
+
 ABSL_FLAG(bool, jsonpathv2, true,
           "If true uses Dragonfly jsonpath implementation, "
           "otherwise uses legacy jsoncons implementation.");

diff --git a/src/server/json_family_test.cc b/src/server/json_family_test.cc
@@ -6,8 +6,6 @@
 
 #include <absl/strings/str_replace.h>
 
-#include <jsoncons/json.hpp>
-
 #include "base/gtest.h"
 #include "base/logging.h"
 #include "facade/facade_test.h"

diff --git a/src/server/search/doc_accessors.cc b/src/server/search/doc_accessors.cc
@@ -12,8 +12,6 @@
 #include <absl/strings/str_cat.h>
 #include <absl/strings/str_join.h>
 
-#include <jsoncons/json.hpp>
-
 #include "base/flags.h"
 #include "core/json/path.h"
 #include "core/overloaded.h"
@@ -86,7 +84,7 @@ SearchDocData BaseAccessor::Serialize(const search::Schema& schema,
   for (const auto& field : fields) {
     const auto& fident = field.GetIdentifier(schema, false);
     const auto& fname = field.GetShortName(schema);
-    
+
     auto field_value =
         ExtractSortableValue(schema, fident, absl::StrJoin(GetStrings(fident).value(), ","));
     if (field_value) {

diff --git a/src/server/test_utils.cc b/src/server/test_utils.cc
@@ -243,6 +243,7 @@ void BaseFamilyTest::ResetService() {
       absl::SetFlag(&FLAGS_alsologtostderr, true);
       fb2::Mutex m;
       shard_set->pool()->AwaitFiberOnAll([&m](unsigned index, ProactorBase* base) {
+        ThisFiber::SetName("Watchdog");
         std::unique_lock lk(m);
         LOG(ERROR) << "Proactor " << index << ":\n";
         fb2::detail::FiberInterface::PrintAllFiberStackTraces();
@@ -359,7 +360,10 @@ bool BaseFamilyTest::WaitUntilCondition(std::function<bool()> condition_cb,
 
 RespExpr BaseFamilyTest::Run(ArgSlice list) {
   if (!ProactorBase::IsProactorThread()) {
-    return pp_->at(0)->Await([&] { return this->Run(list); });
+    return pp_->at(0)->Await([&] {
+      ThisFiber::SetName("Test::Run");
+      return this->Run(list);
+    });
   }
 
   return Run(GetId(), list);