commit 30addc1a5e

Author: kannibalox <kannibalox@gmail.com>
Date:   Mon Jun 10 13:09:29 2024 -0400

    Use regular iterators intead of bucket-local ones

    Trying to manually predict which bucket a hashed key will land in does not
    appear to be well-defined behavior.

    Fixes #1285

src/command_dynamic.cc

@@ -146,7 +146,7 @@ system_method_insert_object(const torrent::Object::list_type& args, int flags) {
   const std::string& rawKey = (itrArgs++)->as_string();
 
   if (rawKey.empty() ||
-      control->object_storage()->find_local(torrent::raw_string::from_string(rawKey)) != control->object_storage()->end(0) ||
+      control->object_storage()->find_raw_string(torrent::raw_string::from_string(rawKey)) != control->object_storage()->end() ||
       rpc::commands.has(rawKey) || rpc::commands.has(rawKey + ".set"))
     throw torrent::input_error("Invalid key.");
 
@@ -355,11 +355,11 @@ system_method_set_function(const torrent::Object::list_type& args) {
   if (args.empty())
     throw torrent::input_error("Invalid argument count.");
 
-  rpc::object_storage::local_iterator itr =
-    control->object_storage()->find_local(torrent::raw_string::from_string(args.front().as_string()));
+  rpc::object_storage::iterator itr =
+    control->object_storage()->find_raw_string(torrent::raw_string::from_string(args.front().as_string()));
 
-  if (itr == control->object_storage()->end(0) || itr->second.flags & rpc::object_storage::flag_constant)
-    throw torrent::input_error("Command is not modifiable.");    
+  if (itr == control->object_storage()->end() || itr->second.flags & rpc::object_storage::flag_constant)
+    throw torrent::input_error("Command is not modifiable.");
 
   return control->object_storage()->set_str_function(args.front().as_string(),
                                                      system_method_generate_command(++args.begin(), args.end()));

src/rpc/object_storage.cc

@@ -61,22 +61,19 @@ const unsigned int object_storage::flag_rlookup;
 
 const size_t object_storage::key_size;
 
-object_storage::local_iterator
-object_storage::find_local(const torrent::raw_string& key) {
-  std::size_t n = hash_fixed_key_type::hash(key.data(), key.size()) % bucket_count();
+object_storage::iterator
+object_storage::find_raw_string(const torrent::raw_string& key) {
+  fixed_key_type<64> k;
+  k.set_c_str(key.data());
 
-  for (local_iterator itr = begin(n), last = end(n); itr != last; itr++)
-    if (itr->first.size() == key.size() && std::memcmp(itr->first.data(), key.data(), key.size()) == 0)
-      return itr;
-
-  return end(bucket_count());
+  return find(k);
 }
 
-object_storage::local_iterator
-object_storage::find_local_const(const torrent::raw_string& key, unsigned int type) {
-  local_iterator itr = find_local(key);
+object_storage::iterator
+object_storage::find_raw_string_const(const torrent::raw_string& key, unsigned int type) {
+  iterator itr = find_raw_string(key);
 
-  if (itr == end(bucket_count()))
+  if (itr == end())
     throw torrent::input_error("Key not found.");
 
   if ((type != 0 && (itr->second.flags & mask_type) != type))
@@ -85,11 +82,11 @@ object_storage::find_local_const(const torrent::raw_string& key, unsigned int ty
   return itr;
 }
 
-object_storage::local_iterator
-object_storage::find_local_mutable(const torrent::raw_string& key, unsigned int type) {
-  local_iterator itr = find_local(key);
+object_storage::iterator
+object_storage::find_raw_string_mutable(const torrent::raw_string& key, unsigned int type) {
+  iterator itr = find_raw_string(key);
 
-  if (itr == end(bucket_count()))
+  if (itr == end())
     throw torrent::input_error("Key not found.");
 
   if ((type != 0 && (itr->second.flags & mask_type) != type) ||
@@ -138,63 +135,63 @@ object_storage::insert(const char* key_data, uint32_t key_size, const torrent::O
 
 bool
 object_storage::has_flag(const torrent::raw_string& key, unsigned int flag) {
-  local_iterator itr = find_local_const(key);
+  iterator itr = find_raw_string_const(key);
   return itr->second.flags & flag;
 }
 
 void
 object_storage::enable_flag(const torrent::raw_string& key, unsigned int flag) {
-  local_iterator itr = find_local_mutable(key);
+  iterator itr = find_raw_string_mutable(key);
   itr->second.flags |= (flag & (flag_constant));
 }
 
 const torrent::Object&
 object_storage::get(const torrent::raw_string& key) {
-  local_iterator itr = find_local_const(key);
+  iterator itr = find_raw_string_const(key);
   return itr->second.object;
 }
 
 const torrent::Object&
 object_storage::set_bool(const torrent::raw_string& key, int64_t object) {
-  local_iterator itr = find_local_mutable(key, flag_bool_type);
+  iterator itr = find_raw_string_mutable(key, flag_bool_type);
   return itr->second.object = !!object;
 }
 
 
 const torrent::Object&
 object_storage::set_value(const torrent::raw_string& key, int64_t object) {
-  local_iterator itr = find_local_mutable(key, flag_value_type);
+  iterator itr = find_raw_string_mutable(key, flag_value_type);
   return itr->second.object = object;
 }
 
 
 const torrent::Object&
 object_storage::set_string(const torrent::raw_string& key, const std::string& object) {
-  local_iterator itr = find_local_mutable(key, flag_string_type);
+  iterator itr = find_raw_string_mutable(key, flag_string_type);
   return itr->second.object = object;
 }
 
 const torrent::Object&
 object_storage::set_list(const torrent::raw_string& key, const torrent::Object::list_type& object) {
-  local_iterator itr = find_local_mutable(key, flag_list_type);
+  iterator itr = find_raw_string_mutable(key, flag_list_type);
   return itr->second.object = torrent::Object::create_list_range(object.begin(), object.end());
 }
 
 void
 object_storage::list_push_back(const torrent::raw_string& key, const torrent::Object& object) {
-  local_iterator itr = find_local_mutable(key, flag_list_type);
+  iterator itr = find_raw_string_mutable(key, flag_list_type);
   itr->second.object.as_list().push_back(object);
 }
 
 const torrent::Object&
 object_storage::set_function(const torrent::raw_string& key, const std::string& object) {
-  local_iterator itr = find_local_mutable(key, flag_function_type);
+  iterator itr = find_raw_string_mutable(key, flag_function_type);
   return itr->second.object = object;
 }
 
 torrent::Object
 object_storage::call_function(const torrent::raw_string& key, target_type target, const torrent::Object& object) {
-  local_iterator itr = find_local_const(key);
+  iterator itr = find_raw_string_const(key);
 
   switch (itr->second.flags & mask_type) {
   case flag_function_type:
@@ -207,13 +204,13 @@ object_storage::call_function(const torrent::raw_string& key, target_type target
 
 bool
 object_storage::has_multi_key(const torrent::raw_string& key, const std::string& cmd_key) {
-  local_iterator itr = find_local_const(key, flag_multi_type);
+  iterator itr = find_raw_string_const(key, flag_multi_type);
   return itr->second.object.has_key(cmd_key);
 }
 
 void
 object_storage::erase_multi_key(const torrent::raw_string& key, const std::string& cmd_key) {
-  local_iterator itr = find_local_mutable(key, flag_multi_type);
+  iterator itr = find_raw_string_mutable(key, flag_multi_type);
 
   itr->second.object.erase_key(cmd_key);
 
@@ -238,7 +235,7 @@ object_storage::set_multi_key_obj(const torrent::raw_string& key, const std::str
   if (!object.is_string() && !object.is_dict_key() && !object.is_list())
     throw torrent::input_error("Object is wrong type.");
 
-  local_iterator itr = find_local_mutable(key, flag_multi_type);
+  iterator itr = find_raw_string_mutable(key, flag_multi_type);
 
   if (itr->second.flags & flag_rlookup) {
     rlookup_iterator r_itr = m_rlookup.find(cmd_key);
@@ -259,7 +256,7 @@ object_storage::rlookup_list(const std::string& cmd_key) {
   torrent::Object::list_type result;
 
   rlookup_iterator r_itr = m_rlookup.find(cmd_key);
-  
+
   if (r_itr != m_rlookup.end())
     std::transform(r_itr->second.begin(), r_itr->second.end(), std::back_inserter(result),
                    std::bind(&key_type::c_str, std::bind(rak::mem_ptr(&value_type::first), std::placeholders::_1)));
@@ -270,7 +267,7 @@ object_storage::rlookup_list(const std::string& cmd_key) {
 void
 object_storage::rlookup_clear(const std::string& cmd_key) {
   rlookup_iterator r_itr = m_rlookup.find(cmd_key);
-  
+
   if (r_itr == m_rlookup.end())
     return;
 

src/rpc/object_storage.h

@@ -79,9 +79,6 @@ public:
   using base_type::size;
   using base_type::empty;
   using base_type::key_eq;
-  using base_type::bucket;
-  using base_type::bucket_count;
-  using base_type::max_bucket_count;
   using base_type::load_factor;
 
   // Verify rlookup is static / const.
@@ -89,7 +86,7 @@ public:
   using base_type::clear;
   using base_type::find;
   using base_type::erase;
-  
+
   static const unsigned int flag_generic_type  = 0x1;
   static const unsigned int flag_bool_type     = 0x2;
   static const unsigned int flag_value_type    = 0x4;
@@ -107,9 +104,9 @@ public:
 
   static const size_t key_size = key_type::max_size;
 
-  local_iterator find_local(const torrent::raw_string& key);
-  local_iterator find_local_const(const torrent::raw_string& key, unsigned int type = 0);
-  local_iterator find_local_mutable(const torrent::raw_string& key, unsigned int type = 0);
+  iterator find_raw_string(const torrent::raw_string& key);
+  iterator find_raw_string_const(const torrent::raw_string& key, unsigned int type = 0);
+  iterator find_raw_string_mutable(const torrent::raw_string& key, unsigned int type = 0);
 
   iterator insert(const char* key_data, uint32_t key_size, const torrent::Object& object, unsigned int flags);
   iterator insert_c_str(const char* key, const torrent::Object& object, unsigned int flags) { return insert(key, std::strlen(key), object, flags); }
@@ -141,7 +138,7 @@ public:
   const torrent::Object& set_string(const torrent::raw_string& key, const std::string& object);
   const torrent::Object& set_c_str_string(const char* str, const std::string& object) { return set_string(torrent::raw_string::from_c_str(str), object); }
   const torrent::Object& set_str_string(const std::string& str, const std::string& object) { return set_string(torrent::raw_string::from_string(str), object); }
-  
+
   const torrent::Object& set_list(const torrent::raw_string& key, const torrent::Object::list_type& object);
   const torrent::Object& set_c_str_list(const char* str, const torrent::Object::list_type& object) { return set_list(torrent::raw_string::from_c_str(str), object); }
   const torrent::Object& set_str_list(const std::string& str, const torrent::Object::list_type& object) { return set_list(torrent::raw_string::from_string(str), object); }